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/0d4a52f44c19a9e891... (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
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/f746c5a9e68d7b4f9dc9c0528596776b3e85b...
)
Ladislav
On Fri, Jan 5, 2024 at 5:36 PM josef Reidinger
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/0d4a52f44c19a9e891... (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
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.
I realized another thing. Running the new Rubocop from a container is quite simple. The same problem is also with running unit tests. That could be also solved by a container or VM. But a worse problem is development and testing. I'm running Leap15 on my machine and if you cannot run the new code in Leap15 then it will be quite difficult to develop and test it on your machine. So the question is who uses Tumbleweed for development and who uses Leap15 (or SLE15)? Unless the majority runs on TW I'd prefer to keep the old backward compatible Ruby 2.5 syntax for easier development and testing. Ladislav -- Ladislav Slezák YaST Developer SUSE LINUX, s.r.o. Corso IIa Křižíkova 148/34 18600 Praha 8
On Thu, 1 Feb 2024 18:31:27 +0100
Ladislav Slezak
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.
I realized another thing. Running the new Rubocop from a container is quite simple. The same problem is also with running unit tests. That could be also solved by a container or VM. But a worse problem is development and testing. I'm running Leap15 on my machine and if you cannot run the new code in Leap15 then it will be quite difficult to develop and test it on your machine.
So the question is who uses Tumbleweed for development and who uses Leap15 (or SLE15)?
Unless the majority runs on TW I'd prefer to keep the old backward compatible Ruby 2.5 syntax for easier development and testing.
Ladislav
Hi, I also developing on Leap 15, but rubocop and unit tests are only parts that does not depend on system. For development and testing you depends on your system tools, libraries and configuration and I worry it will start more and more diverging. I think already some parts of code ( mainly not maintained by us ) uses TW tools, that are no available in SLE15. And I think it will get worse more and more. So I worry that having as goal to be able to develop for TW and ALP on Leap15 can be quite challenging. Josef
-- Ladislav Slezák YaST Developer
SUSE LINUX, s.r.o. Corso IIa Křižíkova 148/34 18600 Praha 8
Hi, I also developing on Leap 15, but rubocop and unit tests are only parts that does not depend on system. For development and testing you depends on your system tools, libraries and configuration and I worry it will start more and more diverging. I think already some parts of code ( mainly not maintained by us ) uses TW tools, that are no available in SLE15. And I think it will get worse more and more. So I worry that having as goal to be able to develop for TW and ALP on Leap15 can be quite challenging.
Hi, This may be a valid use case for distrobox[1]. It sets up a container for you with the linux distribution of your choice (e.g., Tumbleweed) and it can even include systemd if you want to (which makes sense for Agama). Of course, if you need to fake some hardware, a VM is much better. But for some development work, it is worth a try. I have been playing around with it recently, and it does a pretty good job. Regards, Imo [1] https://github.com/89luca89/distrobox
participants (3)
-
Imobach Gonzalez Sosa -
josef Reidinger -
Ladislav Slezak