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 <https://github.com/yast/yast-rake/blob/f746c5a9e68d7b4f9dc9c0528596776b3e85b9c8/lib/tasks/rubocop.rake#L35> ) Ladislav On Fri, Jan 5, 2024 at 5:36 PM josef Reidinger <[email protected]> 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
