chore: upgrade stream_data
This PR also include the following changes:
- [x] Update credo config to the latest format.
- [x] Removes compilation warnings
- [x] Updates the CI workflow to use more recent otp/elixir versions (the
erlef/setup-beamaction can't find the original otp 23.3) - [x] Removes unused
config/config.exsfile - [x] Upgrades all the outdated dependencies
- [x] Fixes all the
credofindings after the upgrade - [x] Fixes some failing unit tests after the upgrade while trying to keep compatibility with previous versions
Can you bump the elixir and Erlang versions in the gha to OTP 25 and whatever the lowest version of elixir is that supports OTP 25
@mhanberg hey 👋 , thanks for taking a look at this. I already applied your suggestion.
Hmm, I'm not sure why the actions runs are not respecting that. I'll take a look once I get back to my computer
I see the results from the Analysis and Tests phase taking the right versions now. But let me look why credo is failing so abruptly and fix some failing unit tests.
I will try to install the otp/elixir combination you suggested locally, probably I will not be able to do it because those are a bit old and I'm currently using an Apple M2, but let me try, I will report back.
oh, i think my phone was just showing the required checks, so the new ones didn't show up
@mhanberg re: credo failures
UPDATE: Credo it's fine up to Elixir 1.16, I was initially locally using v1.17-rc.0 and there is a change is in the elixir tokenizer. ~I will send a fix to credo if I can't find a related PR~. Someone is already taking care of that issue: https://github.com/rrrene/credo/pull/1136
This is a little bit wtf, but here what's happening. If you run mix credo you mostly get these failures:
11:19:48.165 [error] Task #PID<0.254.0> started from #PID<0.216.0> terminating
** (CaseClauseError) no case clause matching: {:ok, 63, 1, [], [{:eol, {62, 4, 1}}, {:end, {62, 1, nil}}, {:eol, {61, 6, 1}}, {:end, {61, 3, nil}}, {:eol, {60, 8, 1}}, {:end, {60, 5, nil}}, {:eol, {59, 63, 1}}, {:")", {59, 62, nil}}, {:"]", {59, 61, nil}}, {:bin_string, {59, 58, nil}, [">"]}, {:",", {59, 56, 0}}, {:")", {59, 55, nil}}, {:identifier, {59, 51, ~c"opts"}, :opts}, {:",", {59, 49, 0}}, {:identifier, {59, 44, ~c"specs"}, :specs}, {:., {59, 43, nil}}, {:identifier, {59, 38, ~c"union"}, :union}, {:"(", {59, 37, nil}}, {:paren_identifier, {59, 31, ~c"to_doc"}, :to_doc}, {:",", {59, 29, 0}}, {:bin_string, {59, 15, nil}, ["#Norm.OneOf<"]}, {:"[", {59, 14, nil}}, {:"(", {59, 13, nil}}, {:paren_identifier, {59, 7, ~c"concat"}, :concat}, {:eol, {58, 32, 1}}, {:do, {58, 30, nil}}, {:")", {58, 28, nil}}, {:identifier, {58, 24, ~c"opts"}, :opts}, {:",", {58, 22, 0}}, {:identifier, {58, 17, ~c"union"}, :union}, {:"(", {58, 16, nil}}, {:paren_identifier, {58, 9, ~c"inspect"}, :inspect}, {:identifier, {58, 5, ~c"def"}, :def}, {:eol, {56, 27, 2}}, {:alias, {56, 20, ~c"Algebra"}, :Algebra}, {:., {56, 19, nil}}, {:alias, {56, 12, ~c"Inspect"}, :Inspect}, {:identifier, {56, 5, ~c"import"}, :import}, {:eol, {55, 21, 1}}, {:do, {55, 19, nil}}, {:alias, {55, 11, ...}, :Inspect}, {:identifier, {55, ...}, :defimpl}, {:eol, {...}}, {:end, ...}, {...}, ...], []}
(credo 1.7.6) lib/credo/code.ex:138: Credo.Code.to_tokens/2
(credo 1.7.6) lib/credo/check/readability/large_numbers.ex:43: Credo.Check.Readability.LargeNumbers.run/2
(credo 1.7.6) lib/credo/check/readability/large_numbers.ex:2: Credo.Check.Readability.LargeNumbers.do_run_on_source_file/3
(elixir 1.17.0-rc.0) lib/task/supervised.ex:101: Task.Supervised.invoke_mfa/2
(elixir 1.17.0-rc.0) lib/task/supervised.ex:36: Task.Supervised.reply/4
Function: &:erlang.apply/2
Args: [#Function<2.75132833/1 in Credo.Check.Readability.LargeNumbers.do_run_on_all_source_files/3>, [%SourceFile<lib/norm/core/any_of.ex>]]
So, I went to the credo code, and certainly noticed the mismatch here:
def to_tokens(source, filename \\ "nofilename") when is_binary(source) do
source
|> String.to_charlist()
|> :elixir_tokenizer.tokenize(1, file: filename)
|> case do
# Elixir < 1.6
{_, _, _, tokens} ->
tokens
# Elixir >= 1.6
{:ok, tokens} ->
tokens
# Elixir >= 1.13
{:ok, _, _, _, tokens} ->
tokens
{:error, _, _, _, tokens} ->
tokens
end
end
So, I did modified that file with this diff:
# diff lib/credo/code.ex.bak lib/credo/code.ex
148c148
< {:ok, _, _, _, tokens} ->
---
> {:ok, _, _, _, tokens, _} ->
Run mix deps.compile --force credo and it worked hahaha
So, let me fix those issues first and then see if credo introduced a bug at some point or whatever.
Ok, now everything passes locally:
Sorry, one last ask. Can you revert any unrelated style changes or credo changes that were due to the checks being changed?
Hey @milmazz! 🖖
Sorry for the ping, but is there any update on this?
If you need (and allow me to), I can open a separate pull request cherry-picking your fixes for the compilation warnings.
This would fix the warnings from compiling other libraries that depend on norm.
@antedeguemon Thanks for the ping! I updated this branch and now we should be ready for another review from @mhanberg
Looks like just needs some credo fixes and I think it's good to go
Those are the credo changes you asked to remove in your previous comment @mhanberg
Those are the credo changes you asked to remove in your previous comment @mhanberg
What i was previously referring to was changing the credo config file so that it added/removed checks, requiring other code changes.
The ones that are currently failing are from updating Credo, as the MapJoin check was added after the previous version of Credo we were on.
These ones are new default credo checks and i'm okay with fixing those.
@mhanberg To move on with this discussion, I put it back the refactoring opportunities found by credo.