Hi, for those who are not interested in details just quick summary. There is new shared defaults for rubocop and it is also automatic detected. So how to quickly switch from old rubocop to new one?
change ``` inherit_from: /usr/share/YaST2/data/devtools/data/rubocop_yast_style.yml ``` to: ``` inherit_from: /usr/share/YaST2/data/devtools/data/rubocop-0.71.0_yast_style.yml ``` And then fix deprecations in old rubocop config and handle new issues arise. To run new rubocop locally install rubygem-rubocop-0_71 and then it is handled by update alternatives or use rake check:rubocop with the latest rubygem-yast-rake. Hint: If you want to have one cop per commit, then run rubocop, pick one of cop that failing and run `rubocop --only <cop>` and fix all instances and commit. So now longer story with more technical details and impressions. Feel free to use this as part of blog post. ## Motivation So lets start with motivation. For me the biggest one is that rubocop in version we used to use support only ruby 2.2, so we cannot use in code any new features of ruby like &. or dig. Of course another one is that previously it was quite old version and newer ones have many new features like more auto corrections, new cops and so on. ## Examples I start with two well tested (bootloader[1], storage-ng[2]) modules to catch possible buggy transformations and with two modules full of ancient converted code ( yast2[3], packager[4]). Feel free to discuss exact configuration of cops so the final yast style fits at least majority of team taste. Ideally we should tune cops instead of disable, so our code is consistent and please always document reasoning for tuning. [1] https://github.com/yast/yast-bootloader/pull/572 [2] https://github.com/yast/yast-storage-ng/pull/934 [3] https://github.com/yast/yast-yast2/pull/946 [4] https://github.com/yast/yast-packager/pull/455 ## Impressions I found some cops really useful especially for killing some y2r zombies like multiline trailing if [1] or identical content in conditional branches [2]. Of course, there is also cops that prefers usage of new features like indented heredoc[3] or safe navigation operator[4]. What also helps me is restriction on short method param names[5] to have meaningful names in method definition as it usually have long scope. The best example is in Progress module parameter "st" which means in half of methods "step" and in second half "stage". Also I learn something new about ruby. As there is now cops that check for void context and this failing in storage-ng which do some fancy things in assign operator. Se let me write here demonstration code: class C def a=(v) "5" end end c = C.new a = C.a = "7" a # is now "7" and not "5" So as you can see return value of writer methods is ignored same as e.g. return value of initialize method. Another cop e.g. warn that URI.escape is deprecated and will be removed, so we can also react to it(sadly in packager I have to disable it as escaping libzypp repo path is more tricky and probably deserve own class).[6] So I think we should more regularly update rubocop as it also prepares us for next ruby versions. [1] https://github.com/yast/yast-yast2/pull/946/commits/5a25810718cb00e5ba567c0666867e6322ffc2ae [2] https://github.com/yast/yast-yast2/pull/946/commits/af60b295e0e383010c676e322e0a09cc414e7412 [3] https://github.com/yast/yast-yast2/pull/946/commits/9c0fb602be0964d3d69aa3717789b5f000468811 [4] https://github.com/yast/yast-yast2/pull/946/commits/96a3ce46ddbfa7e7e97dd6e0be936795af47b58e [5] https://github.com/yast/yast-yast2/pull/946/commits/4d02ff2410173f5851aed8f9324a40d51235fdca [6] https://ruby-doc.org/stdlib-2.4.1/libdoc/uri/rdoc/URI/Escape.html#method-i-escape-label-Description ## Known Issues I found some possible issues. The first one is endless loop auto correct with our combination of cops and having access modifier as first element ( which is done surprising often in storage-ng ). So far I workaround it by using different access modifier indentation in storage-ng[1]. Another endless loop of auto-correct detected by rubocop can be easily fixed manually. The second issue is that using frozen string literal directive which is now encouraged can lead to exceptions. So for yast2 I do not enable it, but for bootloader and storage-ng due to their good test coverage I try to enable it. For bootloader[2] it needs just some small adaptation, but for storage-ng it will need more work as it failing in storage wrapper. It need some code modification. You can ask here why not disable it for all code? Reason is that it will be standard in ruby 3.0 which will arrive soon, so it is more like preparation for future, so whenever it is possible to do, it is good to do it. The third one is found in storage-ng where it tries to change `!(a == b)` but a is self, so it cause ruby endless loop, see [3]. And the forth is more common, now it is preferred to use more secure Yaml.safe_load which do not allow to load classes dump. But we sometimes use it for testing purpose, so it needs to be disabled for that cases[4] [1] https://github.com/yast/yast-storage-ng/pull/934/files#diff-11a0d7906801d9dea0eccb85667ad811R33 [2] https://github.com/yast/yast-bootloader/pull/572/commits/7477371f1f092b74c5090766049508d91201aba2 [3] https://github.com/yast/yast-storage-ng/pull/934/files#diff-e1e2b727919857cfd951890efd74d959R60 [4] https://github.com/yast/yast-storage-ng/pull/934/files#diff-e1e2b727919857cfd951890efd74d959R423 ## Removal of SUSE Style Guide SUSE style guide was removed few months ago with reasoning that we (as SUSE) should follow upstream and if we do not like something, try to convince rubocop default configuration to contain it. So I removed all links from YaST style guide to SUSE one and keep it as YaST tuning. Maybe it is also good time to revisit all things we inherit from SUSE style guide and check if it still fits our needs or if we want to follow upstream. I hope you find new rubocop as useful as me and feel free to comment it as much as you want. Josef -- To unsubscribe, e-mail: [email protected] To contact the owner, e-mail: [email protected]
