The Rubocop changes look reasonable to me.

But as I'm developing on a Leap15 system the ability to run it there is important to me.
Using a container looks like a good idea. As a quick hack it should be possible to use something like

RUBOCOP_BIN="docker/podman run ...." rake check:rubocop

A better solution would be to provide a script wrapper for the alternatives system, then it should work transparently
with a container. (link)

Ladislav


On Fri, Jan 5, 2024 at 5:36 PM josef Reidinger <jreidinger@suse.cz> wrote:
Hi there,
as i work on migrating old configuration to newer rubocop version we support, i also take oppurtinity and some learning time to try the latest rubocop version.

I am creating three experimental pull requests. One is for yast2.rpm, which is a combination of new and old code, storage-ng, which is just new code, and yast2-country, which has no rubocop at all. And let me write here what I see:

yast2 - https://github.com/yast/yast-yast2/pull/1298 only automatic fixes (both -a and -A) are applied and a test fixture needs to be modified as new cop also contains some FileUtils changes.

Storage-ng - https://github.com/yast/yast-storage-ng/pull/1364 is more tricky. As one of the changes is that methods with `?` at the end of the name should return boolean, but in storage there are methods that use nil to indicate not specified. I am not sure which is correct, but we need to be aware of this and perhaps disable it if it is intentional. And also a bug is found in the code https://github.com/yast/yast-storage-ng/pull/1364/commits/0d4a52f44c19a9e89152dae8bf0c8ad1ecb05e0c (it is not revealed as it looks like the method is not called at all).

country - https://github.com/yast/yast-country/pull/321 is a bit bigger, as rubocop was not there before. But thanks to a lot of autocorrection it was not that bad. Unfortunately, I also have a bug in the autocorrection that has already been reported to rubocop. The good thing is that rubocop itself detects it after a change and reports invalid syntax. I also see two infinite loops in the autocorrection, which new rubocop reports quite well, so I improve the code manually and everything works fine.

Overall impression is that after applying new cops code looks better and especially new syntax for named parameters really improve feel of our ruby code so it start looking like modern ruby code. Of course, porting from SLE15 will be more work, but due to many autocorrections I think it will be quite smooth.
Another thing we can do for a smoother migration to the new version is to disable auto-enable of new cops and decide what to enable and what not to enable one by one (of course in the shared rubocop config).
Last but not least, new rubocop does not work on Leap15. So I create my own container with the latest rubocop in which I run it. Another option is to use something like rbenv and for CI just use rpm as CI runs on TW.

We can discuss whether to add it or not on the review, but feel free to comment here if you have any thoughts on it.

Josef


--
--
Ladislav Slezák
YaST Developer

SUSE LINUX, s.r.o.
Corso IIa
Křižíkova 148/34
18600 Praha 8