Fixes #36337 - Update rake and clamp deps #367
Conversation
ofedoren
force-pushed
the
ref-36337-update-rake-clamp
branch
from
6030b04
to
ecead38
Compare
|
[test hammer] |
ofedoren
force-pushed
the
ref-36337-update-rake-clamp
branch
from
ecead38
to
671f1dc
Compare
|
Thanks, @ekohl, updated. |
ofedoren
force-pushed
the
ref-36337-update-rake-clamp
branch
from
671f1dc
to
6da6bd2
Compare
|
Thanks, @ekohl, I've splitted all the changes here to separate commits. I hope this will make review easier. Also, since theforeman/hammer-cli-foreman#613 contains quite similar changes, I'll update that one after this PR gets resolved/approved. |
There was a problem hiding this comment.
https://stackoverflow.com/questions/2098742/ruby-instance-variable-not-initialized-warning/2098835#2098835 suggests the undefined warning goes away with Ruby 3.0. Since you disable warnings anyway, perhaps the last commit should be trimmed down or even skipped.
ofedoren
force-pushed
the
ref-36337-update-rake-clamp
branch
from
6da6bd2
to
3e93338
Compare
And I found the commit with that :) ruby/ruby@01b7d5a So, I've decided to drop the last commit. Hammer is already slow, it's not worth to slow it more. Updated, but without new changes related to uninitialized variables. |
ofedoren
force-pushed
the
ref-36337-update-rake-clamp
branch
from
3e93338
to
8bcd3d4
Compare
| @@ -11,6 +11,7 @@ def initialize(names, description, subcommand_class, options = {}) | |||
| @subcommand_class = subcommand_class | |||
| @hidden = options[:hidden] | |||
| @warning = options[:warning] | |||
| super(@names, @description, @subcommand_class) | |||
There was a problem hiding this comment.
If you call this, I wonder if names, description and subclass_command aren't already stored. I think you can drop @names = etc.
There was a problem hiding this comment.
Not really, AFAIR I've added this line because something wasn't initialized in clamp itself, crashing the commands.
Also, I rather call super with explicit arguments because implicitly it would send params as well, which is not supported by clamp.
UPD: Right, I thought you meant a different thing, my bad.
UPD2: Actually, it seems we still need explicit assignments for these instance variables.
This PR updates
raketo any latest available and updatesclampto be any version, but2.0.0(currently, the latest is 1.3.2).There is also some refactoring due to
clampchanges and some refactoring due torakespamming a lot of Ruby warnings. I fixed majority of them (mostly it wasuninitialized variablewarning, although I'm not sure that's a good idea: https://bugs.ruby-lang.org/issues/17055#note-6)Anyway, there is a new
warningflag forrake'sTestTask, which suppresses all Ruby warnings. Without this flag quite a lot of our tests will fail since they depend on strict output. I leftRUBY_WARNINGSenv variable to be set to run the tests with Ruby warnings.