Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolves deprecation warnings on elixir 1.17 #95

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

PaulOstazeski
Copy link

@PaulOstazeski PaulOstazeski commented Jul 9, 2024

Description

Updates for elixir 1.17

  • Resolves deprecation warnings on elixir 1.17
  • Updates dependencies
  • Updates credo config file

Type of change (check all applicable)

  • 🍕 Feature
  • 🐛 Bug Fix
  • 📝 Documentation Update
  • 🧑‍💻 Code Refactor
  • 🔥 Performance Improvements
  • ✅ Test

Related Tickets & Documents

Fixes #94

Proof of completion, QA Instructions, Screenshots, Recordings

mix test continues passing without substantive changes (in two tests I did relax the match for error messages from strict equality to string matchin)
mix credo continues passing without recommendations

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • Any dependent changes have been merged and published in downstream modules

The error messages in these tests were more verbose, echoing back the
original query. Rather than expand the expected string, matching on the
more relevant portion of the observed string (including the error codes)
seemed more tenable.

I'm also uncertain if the version (or a setting) of mysql may impact the
verbosity of the returned error message.
* Logger has changed "warn" to "warning" level
* "module.__function__" now outputs a warning message about implicit
  function calls, suggests using "module.__function__()"
* Exception.exception?/1 replaced by Kernel.is_exception/1
* Regex.regex?/1 replaced by Kernel.is_struct/2
* Some ecto callbacks take different numbers of parameters, so
  TestAdapter no-op function implementations updated.
Regenerate credo config, keeping opted-in checks, using very slightly
relaxed parameters for duplicated code and nesting (both 3 over defaults
of 2).
@@ -37,13 +37,17 @@ defmodule TriplexTest do
test "create/2 must return a error if the tenant already exists" do
assert {:ok, _} = Triplex.create("lala", PGTestRepo)

assert {:error, "ERROR 42P06 (duplicate_schema) schema \"lala\" already exists"} =
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This failed for me because the message returned was "ERROR 42P06 (duplicate_schema) schema \"lala\" already exists\n\n query: create schema \"lala\";" - I'm skeptical that is a result of using elixir 1.17 and suspect it is caused by either my version of mysql being different or a configuration setting in my mysql. In my opinion the "meat" of the assertion is preserved in that the error code is still checked.

{Credo.Check.Refactor.MatchInCondition, []},
{Credo.Check.Refactor.NegatedConditionsInUnless, []},
{Credo.Check.Refactor.NegatedConditionsWithElse, []},
{Credo.Check.Refactor.Nesting, [max_nesting: 3]},
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default here is 2.

With the default, mix credo suggests that lib/mix/triplex.ex:109 and lib/triplex.ex:273 are too deeply nested. Rather than refactor those, loosening the credo setting by one level avoids "extra" changes in this PR.

I'd be happy to attempt refactoring those and resetting this to max_nesting: 2.

#
{Credo.Check.Design.AliasUsage,
[priority: :low, if_nested_deeper_than: 2, if_called_more_often_than: 0]},
{Credo.Check.Design.DuplicatedCode, [nodes_threshold: 3]},
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default here is 2.

With the default, mix credo suggests that lib/mix/tasks/triplex.mysql.install.ex:48 and lib/mix/tasks/triplex.gen.migration.ex:78 are duplicates. Since those tasks generate code, it seems to me to be preferable to keep that duplication.

An alternative would be to use the default credo config here and add inline credo disable comments to those files. Another alternative would be increase the mass_threshold setting for this check.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Compile warnings in Elixir 1.17
1 participant