Fixes/invalid yield #321
Fixes/invalid yield #321
Conversation
lib/metric_fu/loader.rb
Outdated
| @@ -27,14 +27,15 @@ def lib_require(base = "", &_block) | |||
| # Adds methods x_dir and _x_require for the directory x, | |||
| # relative to the metric_fu lib, to the given klass | |||
| # @param klass [Class] the klass to add methods for the specified directories | |||
| # @param library_dirs [Array] list of directories | |||
There was a problem hiding this comment.
@etagwerker I think you can remove this comment line, library_dirs is not a param of this method.
That idiom is no longer supported. I decided to go with a constant.
etagwerker
force-pushed
the
fixes/invalid-yield
branch
from
ce6f6b1
to
074c715
Compare
|
@lubc I just removed that old comment. I thought I was going to need it. In the end I decided to implement the patch with a different approach. 👍 |
| @@ -34,7 +34,7 @@ def lib_require(base = "", &_block) | |||
| # ::metrics_require which takes a block of files to require relative to the metrics_dir | |||
| def create_dirs(klass) | |||
| class << klass | |||
| Array(yield).each do |dir| | |||
There was a problem hiding this comment.
did it just need a create_dirs(klass, &block) instead maybe?
There was a problem hiding this comment.
yeah, I guess that could have worked too. I thought it was a good opportunity to simplify unnecessary indirection...
There was a problem hiding this comment.
I'm not sure anyone cares, but this fix changes semantics. One could, with the old code, override the library_dirs in use without a warning of overriding a constant (likewise for artifact_subdirs, I think). To this fixes credit, it is simpler. I don't know if any of these directories are subject to the whim of the tools metric_fu calls. If they are, then keeping the old semantics seems better.
|
@etagwerker @lubc Is this change released in rubygems? 🤝 |
|
@pedrofurtado hey Pedro! I don't think so. I'll talk to my team tomorrow to see if we can release a new version 👍 |
|
@lubc Thanks a lot! |
Hi there,
This PR fixes #319 because the current syntax does not work with Ruby 3.0
Please check it out.
Thanks!