Resolves deprecation warnings on elixir 1.17 #95
Conversation
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).
| 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"} = |
There was a problem hiding this comment.
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]}, |
There was a problem hiding this comment.
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]}, |
There was a problem hiding this comment.
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.
|
Hi, this change would be extremely helpful for me as it would clean up my log. Would it be possible to merge it? Thank you for considering it! |
|
@panjan I'm skeptical this will be merged, as there's been very little activity on triplex in some time. I might consider switching to a fork. I note that https://github.com/aseigo/triplex exists, but I have not tried it myself so I make no claims beyond that. |
|
@PaulOstazeski thank you for your comments and thanks for taking the time to reply. I’ll definitely take a look at the fork you mentioned. If I manage to get things working, I’ll leave an update here in case it helps others too. |
|
please consider making merges again @kelvinst @engedmundo ? |
|
well, looks like triplex is not really being maintained... maybe they should add more collaborators |
4 participants
Description
Updates for elixir 1.17
Type of change (check all applicable)
Related Tickets & Documents
Fixes #94
Proof of completion, QA Instructions, Screenshots, Recordings
mix testcontinues passing without substantive changes (in two tests I did relax the match for error messages from strict equality to string matchin)mix credocontinues passing without recommendationsChecklist: