New rubocop (0.41.2 → 1.24.1) #603
Conversation
src/lib/registration/registration.rb
Outdated
| log.info "Found obsoleted service: #{old_service}" | ||
| ::Registration::SwMgmt.remove_service(old_service) | ||
| end | ||
| if old_service && !old_service.empty? && old_service != product_service.name && ::Registration::SwMgmt.service_installed?(old_service) |
There was a problem hiding this comment.
Is it intentional to delete the comment here?
There was a problem hiding this comment.
Sorry, I do not see on github which comment is removed?
.rubocop.yml
Outdated
| @@ -73,3 +73,8 @@ Style/OptionalBooleanParameter: | |||
| # it sometimes make code harder to read | |||
| Style/GuardClause: | |||
| Enabled: false | |||
|
|
|||
| # disable missing ending range as it is not supported in ruby 2.7 | |||
| # TODO: is it bug in rubocop that with target ruby version 2.7 it stil do it? | |||
There was a problem hiding this comment.
Interesting question.
The current source for the cop thinks endless ranges should work even since Ruby 2.6:
https://github.com/rubocop/rubocop/blob/751898494b694f0520d1739b877e1add94c71366/lib/rubocop/cop/style/slicing_with_range.rb
There was a problem hiding this comment.
Ah, it is my fault. ruby in SLE15 is 2.5 and not 2.7.
jreidinger@jreidinger:~/prace/yast/registration> ruby --version
ruby 2.5.9p229 (2021-04-05 revision 67939) [x86_64-linux-gnu]
| @@ -311,13 +308,10 @@ def service_for_product(product, &block) | |||
| def update_services(product_service) | |||
| old_service = product_service.obsoleted_service_name | |||
| # sanity check | |||
| if old_service && !old_service.empty? && old_service != product_service.name | |||
| # old_service comes from SCC. So it could be that we have already removed | |||
test/spec_helper.rb
Outdated
| @@ -106,7 +106,7 @@ def load_resolvable(filename) | |||
| require_relative "factories" | |||
|
|
|||
| # force loading all files to report proper code coverage | |||
| Dir.chdir(libdir) { Dir["**/*.rb"].each { |f| require f } } | |||
| Dir.chdir(libdir) { Dir["**/*.rb"].sort.each { |f| require f } } | |||
There was a problem hiding this comment.
I think this forced loading is not needed anymore since we use the SimpleCov.track_files call above.
| "but the server connection still cannot be trusted.\n\n" \ | ||
| "Please fix the certificate issue manually, ensure that the server\n" \ | ||
| "can be connected securely and start the YaST module again.")) | ||
| "but the server connection still cannot be trusted.\n\n" \ |
There was a problem hiding this comment.
I'm not sure if I like this extra indentation for texts. It makes less space available and requires more shorter lines. 🤔
Esp. it is strange that for Hashes or named parameters it requires the opposite.
| urltok: {}, destdir: "") | ||
| urlpath: "/#{name}", | ||
| localfile: path, | ||
| urltok: {}, destdir: "") |
There was a problem hiding this comment.
This example of an inconsistency, for multiline string it would require the extra indentation here.
Co-authored-by: Martin Vidner <mvidner@suse.cz>
|
Dropping that "require all" revealed a lot of missing |
|
@jreidinger @lslezak IMHO we're too strict on ourselves by removing the "require everything" bit in tests. It can wait for another housekeeping round. Or I can clean it up if you feel that now the time is right. |
|
all is green, so ready for final blessing. |
|
✔️ Internal Jenkins job #118 successfully finished |
Problem
The old rubocop version used for style no longer supports new ruby (3.3 used in Tumbleweed)
Solution
Update to the latest predefined yast version of rubocop.