[yast-devel] Codeclimate update

newer
[yast-devel] Pending Pull Requests

older
[yast-devel] Pending Pull Requests

Ancor Gonzalez Sosa

29 Nov 2017 29 Nov '17

15:24

In the yast-storage-ng repository we noticed that Codeclimate became more picky than it used to be and turns that, indeed, they have done a big update to the service introducing many new checks and even a new format for the configuration file codeclimate.yml. The old format still works, so we don't have to change anything unless we want to configure some of the new checks. And that's the case of yast-storage-ng. Codeclimate was complaining because a method was 26 lines long, but we have configured Rubocop to accept up to 30. So I created this pull request to convert the configuration file to the new format and to sync the settings there with our Rubocop settings: https://github.com/yast/yast-storage-ng/pull/441/commits/1ffd50908f5b8289d84... The point is that there is another new check that is yelling to us and, since there is no 1:1 Rubocop equivalent we need to take a decision. Codeclimate thinks that each class should not have more than 20 methods. With my pull request, Y2Storage::Md has grown to 23 methods. What should we do? Try to honor this new check (which means rethinking some of our classes, MD is simply today's example)? Disable it completely? Raise the threshold to any value you guys find reasonable? Input required to continue with that PR. Cheers. -- Ancor González Sosa YaST Team at SUSE Linux GmbH -- To unsubscribe, e-mail: yast-devel+unsubscribe@opensuse.org To contact the owner, e-mail: yast-devel+owner@opensuse.org

Show replies by date

Steffen Winterfeldt

29 Nov 29 Nov

15:45

On Wednesday 2017-11-29 16:24, Ancor Gonzalez Sosa wrote:

...

The point is that there is another new check that is yelling to us and, since there is no 1:1 Rubocop equivalent we need to take a decision. Codeclimate thinks that each class should not have more than 20 methods. With my pull request, Y2Storage::Md has grown to 23 methods.

What should we do? Try to honor this new check (which means rethinking some of our classes, MD is simply today's example)? Disable it completely? Raise the threshold to any value you guys find reasonable?

I would not spend too much time with this. Just disable it. Steffen -- To unsubscribe, e-mail: yast-devel+unsubscribe@opensuse.org To contact the owner, e-mail: yast-devel+owner@opensuse.org

Stefan Hundhammer

16:19

On 29.11.2017 16:24, Ancor Gonzalez Sosa wrote:

...

Codeclimate thinks that each class should not have more than 20 methods. With my pull request, Y2Storage::Md has grown to 23 methods.

What should we do?

Ditch that "tool". If it imposes yet another set of insane restrictions upon us, it contributes to our problems, not to any solutions. What exactly does Codeclimate do that would warrant bothering with it in the first place? Kind regards -- Stefan Hundhammer YaST Developer SUSE Linux GmbH GF: Felix Imendörffer, Jane Smithard, Graham Norton; HRB 21284 (AG Nürnberg) Maxfeldstr. 5, 90409 Nürnberg, Germany -- To unsubscribe, e-mail: yast-devel+unsubscribe@opensuse.org To contact the owner, e-mail: yast-devel+owner@opensuse.org

Stefan Hundhammer

16:26

On 29.11.2017 17:19, Stefan Hundhammer wrote:

...

What exactly does Codeclimate do that would warrant bothering with it in the first place?

https://codeclimate.com/blog/code-climate-platform "Rubocop - Style and quality checks for Ruby code (including support for RRuby 2.2+)" Does it do anything else that is relevant for us beyond calling rubocop? I recall that we were looking for more and better Ruby checkers several times, but we came up empty. I recall asking that question on [research] (upon request) and not getting any result other than "use rubocop" and of course the usual side-tracking discussions in that thread. So if it's just another front-end to rubocop, why are we using it? We use rubocop anyway, so what else does it give us? Kind regards -- Stefan Hundhammer YaST Developer SUSE Linux GmbH GF: Felix Imendörffer, Jane Smithard, Graham Norton; HRB 21284 (AG Nürnberg) Maxfeldstr. 5, 90409 Nürnberg, Germany -- To unsubscribe, e-mail: yast-devel+unsubscribe@opensuse.org To contact the owner, e-mail: yast-devel+owner@opensuse.org

Ladislav Slezak

30 Nov 30 Nov

12:55

Dne 29.11.2017 v 17:26 Stefan Hundhammer napsal(a):

...

So if it's just another front-end to rubocop, why are we using it? We use rubocop anyway, so what else does it give us?

Josef already mentioned some. I'd just add that CC does not provide any feature without which we could not live. It's an additional tool, so far we use it in few packages. For small fixes in the code it is not much useful. Where I see the value is when you want to do a bigger refactoring. It can easily point you to complicated classes/methods, duplicates, etc. And if you do a refactoring you can see how much you improved the code (removed X duplicates, Y less complex methods). Another thing is that it tracks the history so you can see whether the project is improving or becoming worse and worse in the long term. Unfortunately the history at CC is limited to 6 months, for more you have to pay... :-/ -- Ladislav Slezák YaST Developer SUSE LINUX, s.r.o. Corso IIa Křižíkova 148/34 18600 Praha 8 -- To unsubscribe, e-mail: yast-devel+unsubscribe@opensuse.org To contact the owner, e-mail: yast-devel+owner@opensuse.org

José Iván López González

29 Nov 29 Nov

17:54

At this moment I would simply raise the threshold. When time will allow it, we can think about it, but for now I would increase it a little bit (25-30 ?). I would avoid to disable it completely. We need somebody/something who remembers us we can do it better :) On 11/29/2017 03:24 PM, Ancor Gonzalez Sosa wrote:

...

In the yast-storage-ng repository we noticed that Codeclimate became more picky than it used to be and turns that, indeed, they have done a big update to the service introducing many new checks and even a new format for the configuration file codeclimate.yml. The old format still works, so we don't have to change anything unless we want to configure some of the new checks.

And that's the case of yast-storage-ng. Codeclimate was complaining because a method was 26 lines long, but we have configured Rubocop to accept up to 30. So I created this pull request to convert the configuration file to the new format and to sync the settings there with our Rubocop settings: https://github.com/yast/yast-storage-ng/pull/441/commits/1ffd50908f5b8289d84...

The point is that there is another new check that is yelling to us and, since there is no 1:1 Rubocop equivalent we need to take a decision. Codeclimate thinks that each class should not have more than 20 methods. With my pull request, Y2Storage::Md has grown to 23 methods.

What should we do? Try to honor this new check (which means rethinking some of our classes, MD is simply today's example)? Disable it completely? Raise the threshold to any value you guys find reasonable?

Input required to continue with that PR.

Cheers.

-- To unsubscribe, e-mail: yast-devel+unsubscribe@opensuse.org To contact the owner, e-mail: yast-devel+owner@opensuse.org

Stefan Hundhammer

30 Nov 30 Nov

10:07

On 29.11.2017 18:54, José Iván López González wrote:

...

I would avoid to disable it completely. We need somebody/something who remembers us we can do it better :)

The question remains: What exactly does it do for us? If it's really just another front-end to rubocop, it's not worth the hassle. Using a web service that promises to be a tool just for the sake of using it does not give us any benefit; there has to be something that the other tools we are already using don't do. Is there something that the others don't do? Kind regards -- Stefan Hundhammer YaST Developer SUSE Linux GmbH GF: Felix Imendörffer, Jane Smithard, Graham Norton; HRB 21284 (AG Nürnberg) Maxfeldstr. 5, 90409 Nürnberg, Germany -- To unsubscribe, e-mail: yast-devel+unsubscribe@opensuse.org To contact the owner, e-mail: yast-devel+owner@opensuse.org

Josef Reidinger

10:14

On Thu, 30 Nov 2017 11:07:43 +0100 Stefan Hundhammer wrote:

...

On 29.11.2017 18:54, José Iván López González wrote:

...
I would avoid to disable it completely. We need somebody/something who remembers us we can do it better :)

The question remains: What exactly does it do for us? If it's really just another front-end to rubocop, it's not worth the hassle.

Using a web service that promises to be a tool just for the sake of using it does not give us any benefit; there has to be something that the other tools we are already using don't do. Is there something that the others don't do?

Kind regards

Well, advantage against plain rubocop is that it 1. integrate more services - see https://docs.codeclimate.com/docs/list-of-engines 2. visualize it 3. have rating, so it is not plain pass/fail, but allow to see something like worse, but still not above limit. 4. provides badge for project, so it shows external people some idea about code quality Josef -- To unsubscribe, e-mail: yast-devel+unsubscribe@opensuse.org To contact the owner, e-mail: yast-devel+owner@opensuse.org

Arvin Schnell

10:37

On Thu, Nov 30, 2017 at 11:14:46AM +0100, Josef Reidinger wrote:

...

On Thu, 30 Nov 2017 11:07:43 +0100 Stefan Hundhammer wrote:

...
On 29.11.2017 18:54, José Iván López González wrote:

...
I would avoid to disable it completely. We need somebody/something who remembers us we can do it better :)

The question remains: What exactly does it do for us? If it's really just another front-end to rubocop, it's not worth the hassle.

Using a web service that promises to be a tool just for the sake of using it does not give us any benefit; there has to be something that the other tools we are already using don't do. Is there something that the others don't do?

Kind regards

Well, advantage against plain rubocop is that it

...

4. provides badge for project, so it shows external people some idea about code quality

Wow! How about releasing bugzilla statistics so users can see if a project also actually works. ciao Arvin -- Arvin Schnell, Senior Software Engineer, Research & Development SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) Maxfeldstraße 5 90409 Nürnberg Germany -- To unsubscribe, e-mail: yast-devel+unsubscribe@opensuse.org To contact the owner, e-mail: yast-devel+owner@opensuse.org

Stefan Hundhammer

10:48

On 30.11.2017 11:14, Josef Reidinger wrote:

...

Well, advantage against plain rubocop is that it

1. integrate more services - seehttps://docs.codeclimate.com/docs/list-of-engines

Yes, I looked at that, and what Ruby-relevant things I see beyond rubocop is: - brakeman: checks RAILS apps for security vulnerabilities We don't use RAILS, so this does not appear to be relevant for us. - bundler-audit: Find security vulnerabilities in your Ruby dependencies. I would hope that our security team keeps looking for those things in the entire distribution. Other Ruby development teams outside of SUSE don't have that luxury, so that might really be a plus for them. - codeclimate-duplication: Tries to find duplicate code. OK, that might be useful if it really works. But the question is if it works across package boundaries, e.g. between one YaST package and others. I am doubtful about that.

...

2. visualize it

https://codeclimate.com/github/yast/yast-storage-ng Is that it, or is there more?

...

3. have rating, so it is not plain pass/fail, but allow to see something like worse, but still not above limit.

https://codeclimate.com/github/yast/yast-storage-ng https://codeclimate.com/github/yast/yast-installation https://codeclimate.com/github/yast/yast-network Hm...

...

4. provides badge for project, so it shows external people some idea about code quality

If it's badges we are after, I can offer you as many as you like - colored, black and white, whatever. But I am not sure how many users and customers care about that. As Arvin mentioned, the number and severity of bugs and our response time for a first reaction and until a fix is released will matter a lot more to them. But none of that is something we make easily accessible to the public. Just my 2 Cents. Kind regards -- Stefan Hundhammer YaST Developer SUSE Linux GmbH GF: Felix Imendörffer, Jane Smithard, Graham Norton; HRB 21284 (AG Nürnberg) Maxfeldstr. 5, 90409 Nürnberg, Germany -- To unsubscribe, e-mail: yast-devel+unsubscribe@opensuse.org To contact the owner, e-mail: yast-devel+owner@opensuse.org

Josef Reidinger

11:32

On Thu, 30 Nov 2017 11:48:45 +0100 Stefan Hundhammer wrote:

...

On 30.11.2017 11:14, Josef Reidinger wrote:

...
Well, advantage against plain rubocop is that it

1. integrate more services - seehttps://docs.codeclimate.com/docs/list-of-engines

Yes, I looked at that, and what Ruby-relevant things I see beyond rubocop is:

- brakeman: checks RAILS apps for security vulnerabilities

We don't use RAILS, so this does not appear to be relevant for us.

- bundler-audit: Find security vulnerabilities in your Ruby dependencies.

I would hope that our security team keeps looking for those things in the entire distribution. Other Ruby development teams outside of SUSE don't have that luxury, so that might really be a plus for them.

- codeclimate-duplication: Tries to find duplicate code. OK, that might be useful if it really works. But the question is if it works across package boundaries, e.g. between one YaST package and others. I am doubtful about that.

There is also reek one (in second table), which have some interesting checks - https://github.com/troessner/reek . It can be also configured to track your test coverage ( so it can be used instead of coveralls )

...

...
2. visualize it

https://codeclimate.com/github/yast/yast-storage-ng

Is that it, or is there more?

well, it shows - proportion of ratings of files ( so you know if there is few bad files and rest is good, or bunch of average rated files ) - there are summary - there are trends in three dimensions ( e.g. churn one is interesting, also remaining ones ) - there are list of issues - there are progress report so you see what changes during last month - there are overview of per file ratings - it is connected together, so you can easily click thrue it ( not so easy for rubocop on CLI ) What do you missing here in visualization?

...

...
3. have rating, so it is not plain pass/fail, but allow to see something like worse, but still not above limit.

https://codeclimate.com/github/yast/yast-storage-ng https://codeclimate.com/github/yast/yast-installation https://codeclimate.com/github/yast/yast-network

Hm...

Sorry, I have not mind reader. I have no idea what you dislike here.

...

...
4. provides badge for project, so it shows external people some idea about code quality

If it's badges we are after, I can offer you as many as you like - colored, black and white, whatever.

But I am not sure how many users and customers care about that. As Arvin mentioned, the number and severity of bugs and our response time for a first reaction and until a fix is released will matter a lot more to them. But none of that is something we make easily accessible to the public.

Badges are not for users nor customers. It is for external contributors to get idea about project state like code quality, test coverage, documentation coverage quickly. It is current trend for public project to show quickly its state. See e.g. trending projects on github like: https://github.com/antvis/g2 https://github.com/tldr-pages/tldr So we can be dinosaur like COBOL and avoid any trends and changes and become legacy that no-one wanted to touch. Or we can try to look around, adapt to trends and try to attract some attention.

...

Just my 2 Cents.

Kind regards

-- To unsubscribe, e-mail: yast-devel+unsubscribe@opensuse.org To contact the owner, e-mail: yast-devel+owner@opensuse.org

Ladislav Slezak

12:08

Dne 30.11.2017 v 11:48 Stefan Hundhammer napsal(a):

...

On 30.11.2017 11:14, Josef Reidinger wrote: ... - codeclimate-duplication: Tries to find duplicate code. OK, that might be useful if it really works. But the question is if it works across package boundaries, e.g. between one YaST package and others. I am doubtful about that.

That works very well, it actually uses some fuzzy search, i.e. if you copy a block and slightly modify it (rename variables, change constants, call a different method...) then it will still complain about a similar piece of code. https://codeclimate.com/github/yast/yast-storage-ng/issues?engine_name%5B%5D... Of course, it does not work across the repository boundary so it cannot see the external duplicates. I wanted to run some duplication scan in YaST locally, but I do not know which tool they use, maybe it's their own internal tool.

...

...
4. provides badge for project, so it shows external people some idea about code quality

If it's badges we are after, I can offer you as many as you like - colored, black and white, whatever.

What's wrong with badges? When I want to use some rubygem and it has a codeclimate badge (usually displaying a good value otherwise they would not use it I guess ;-)) then it encourages me to use it. Or I might pick a different gem... The same with the Travis badge, if it is there it means the developers take testing (more) seriously...

...

But I am not sure how many users and customers care about that. As Arvin mentioned, the number and severity of bugs and our response time for a first reaction and until a fix is released will matter a lot more to them. But none of that is something we make easily accessible to the public.

Of course, the badge is not designed for the end users. Our end users also do not look at bugzilla (until they find an issue). But there is no point in discussing it, you just put a link to README and that's it. It's for free. -- Ladislav Slezák YaST Developer SUSE LINUX, s.r.o. Corso IIa Křižíkova 148/34 18600 Praha 8 -- To unsubscribe, e-mail: yast-devel+unsubscribe@opensuse.org To contact the owner, e-mail: yast-devel+owner@opensuse.org

Ladislav Slezak

12:33

Dne 29.11.2017 v 16:24 Ancor Gonzalez Sosa napsal(a):

...

In the yast-storage-ng repository we noticed that Codeclimate became more picky than it used to be and turns that, indeed, they have done a big update to the service introducing many new checks and even a new format for the configuration file codeclimate.yml. The old format still works, so we don't have to change anything unless we want to configure some of the new checks.

And that's the case of yast-storage-ng. Codeclimate was complaining because a method was 26 lines long, but we have configured Rubocop to accept up to 30. So I created this pull request to convert the configuration file to the new format and to sync the settings there with our Rubocop settings: https://github.com/yast/yast-storage-ng/pull/441/commits/1ffd50908f5b8289d84...

What I do not like is that there is a duplication in .rubocop.yml, and .codeclimate.yml, ideally the settings should be shared. Our config disables Rubocop because we refer to the shared config stored at /usr/share/YaST2/... which of course is missing at CC. Just a speculation: Maybe if Rubocop is enabled it could read it's config so we would not need the duplication. Rubocop allows downloading the config via HTTP, so we could enable it after changing the config location to the GitHub URL [1]. See [2] for more details. But another problem is that they run Rubocop 0.46.0 by default while we use 0.41.2. You can configure a different version [3], but the oldest supported version unfortunately is 0.42 [4]. But maybe it would be enough, so if you have some spare time you could try that.

...

What should we do? Try to honor this new check (which means rethinking some of our classes, MD is simply today's example)? Disable it completely? Raise the threshold to any value you guys find reasonable?

It depends on you how much you would like to use the tool. But for me, I'd raise the limit. If CC is too annoying for you then you can disable evaluating pull requests (keep CC enabled, just do not block PR). [1] https://raw.githubusercontent.com/yast/yast-devtools/master/ytools/y2tool/ru... [2] http://rubocop.readthedocs.io/en/latest/configuration/#inheriting-configurat... [3] https://docs.codeclimate.com/docs/rubocop#section-using-rubocop-s-newer-vers... [4] https://github.com/codeclimate/codeclimate-rubocop/branches/all?utf8=%E2%9C%93&query=channel%2Frubocop -- Ladislav Slezák YaST Developer SUSE LINUX, s.r.o. Corso IIa Křižíkova 148/34 18600 Praha 8 -- To unsubscribe, e-mail: yast-devel+unsubscribe@opensuse.org To contact the owner, e-mail: yast-devel+owner@opensuse.org

2356

Age (days ago)

2357

Last active (days ago)

List overview

Download

12 comments

7 participants

participants (7)

Ancor Gonzalez Sosa
Arvin Schnell
Josef Reidinger
José Iván López González
Ladislav Slezak
Stefan Hundhammer
Steffen Winterfeldt

[yast-devel] Codeclimate update

tags

participants (7)