Mailinglist Archive: yast-devel (48 mails)

< Previous Next >
[yast-devel] Hackweek project: Welcome New Rubocop
  • From: Josef Reidinger <jreidinger@xxxxxxx>
  • Date: Fri, 28 Jun 2019 13:29:43 +0200
  • Message-id: <20190628132943.5cdbbf86@linux-vvcf.privatesite>
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?







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.


## 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
Se let me write here demonstration code:

class C
def a=(v)

c =
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


## 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]


## 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.

To unsubscribe, e-mail: yast-devel+unsubscribe@xxxxxxxxxxxx
To contact the owner, e-mail: yast-devel+owner@xxxxxxxxxxxx

< Previous Next >
List Navigation
This Thread
  • No further messages