PROPOSAL: automate linting pre-commit or server hooks approach

[oca]

PROPOSAL: automate linting pre-commit or server hooks approach

[dreispt]

I don't know yapf, but AFAICS it's a code formatting tool.
What do you mean with the "format war"? As long as it passes lint checks, no specific code formatting is required from contributors.

IMO formatting tools usage is a personal decision, and OCA should not "adopt" any tools.
I personally don't use them, but I agree it can help new contributors to reformat existing production code to fit OCA standards.

Having said that, this repo welcomes contributions for tools can be help developers to produce better code. I should point out that there is already autopep8 tool, dit you try it?

[blaggacao]

@dreispt Thanks for your reply and opinion! I personally stumbled over yapf, because it adopts the magnificent golang's gofmt and because it's actively maintained by google. The workflow is something like this: On saving the file for example in Sublime Text, everything goes auto formatted.

The "format war" refers to the lint checks, read: "me alone against the lint check machine"... I feel this is brainpower better to be used for more "peaceful" and productive activities... 😄

I think my anaconda sublime text IDE uses Autopep8, there is a keyboard shortcut. However, if you have tried once the effects of gofmt, the experience is a lot better... In every go program you write, at saving time, everything is formatted, sorted, etc. There is no single possibility to get things wrong, without even having thought the topic one single time. In the go world it is just a non-topic.

My proposal goes a bit in this direction, try to achieve it to be a non-topic-at-all for all the valuable contributors who try their best, but sometimes no so knowledgeable effort, to improve things. Could we achieve this somehow?

It is based on my personal experience that during a quick supposedly "no-brainer" doc improvement, I proposed, we closed it because, I wasn't able/willing to dedicate so many extra retakes on that kind of topics. Finally, we closed it and the contribution remains lost in the mean time and can do no good to future users. Part of it attribuible to formatting. That's sad, but it's not about lack of commitment, etc. it's about shortcomings in appropriate tools, I think.

[danimaribeiro]

In my example, I use autopep8 in eclipse, so I configured to format automatically on save.
But there are lot of IDE, text editor and so on, it's hard to adopt just one tool to do this.

[blaggacao]

You're right, it's the benevolent dictator's (Guido's) decision to let us suffer... So we keep suffering. 😄

Maybe change the proposal to dedicate a doc to "praparing your dev environment". Undoubtely, there are efficiency benefits if we converge to one way. I'm working on a plattform independent docker dev environment for example, this could be part of it. Closing and reopening with different title.

[danimaribeiro]

I think "Preparing your dev environment" is a nice to have documentation. There will be the best place to put these kind of tool.

[dreispt]

The Maintainer Tools wiki sounds like the perfect place for that.

[blaggacao]

I reopen this, because thinking it over lunchtime and discussing with a collegue, this is wonderfully suitable for a github server hook, so on each PR, a script automatically pipes everything through PyYapf and ammends to the commit. Like this, style is solved FOREVER 👍 😄

[lasley]

I don't know if I would implement automation of this sort on a repo server side. Seems like it might cause a lot of unintended side-effects.

IMHO styling should be handled by the developer using whatever methods they choose. As I understand it, client side hooks are typically the norm for this.

[blaggacao]

@lasley thanks for you comment! My opinion is, that this is mainly a question of habit. Have you heard about gofmt tool in the golang ecosystem? I've tried it recently and I want this feature so bad for us odooers. It's just genious. This last comment comes closest to this gofmt user experience.

There are indeed unintended side-effects on large litterals, but on the other hand PyYapf has a simple syntax for disableing on expressedly manually formatted code.

# disable: pyyapf
 some code
# enable: pyyapf

I think this is acceptable for the promised benefits, don't you think?

[lasley]

@blaggacao - I do agree that gofmt is awesome, and I love reading Golang due to it.

My only reservation here is that Gofmt is stdlib for Go, and it was designed right alongside the language. I haven't used PyYapf, so I can't comment on it's inner-workings/accuracy, but anything automated on a repo that's not stdlib scares me.

I am very new to Odoo, so maybe this is a bigger problem than I realize, but most of the OCA code I've read looks great (assuming the repo is passing Travis). I'll yield to you more experienced Odooers for sure, was just tossing in my opinion 😄

[blaggacao]

Absolutely, there are sure risks, but I think the apple is feisty and red...
Pyyapf seems to have implementation for Google, Facebook, and Chromium...

https://github.com/google/yapf/blob/master/yapf/yapflib/style.py#L175

So I suppose, it is successfully used by a lot of developers. Maybe git commit --ammend would not be right, but an automated linting commit. Like this, we do not go any risk, by conserving history at the cost of a slightly incresed clutter... (although very well filterably as to standard wording)...

[blaggacao]

cc/ @pedrobaeza @moylop260 because of your work on https://github.com/OCA/maintainer-quality-tools/tree/master/git and OCA/maintainer-quality-tools#242

• What would be your opinion about yapf?
• What outcome would a thorough analysis of the possibility of server hooks yield?

You are the experts in this field... 😄

[blaggacao]

@lasley Something that caters to your objections and kind of leverages yelp community, which could be integrated:
http://pre-commit.com/

[pedrobaeza]

Well, I don't like auto-formating as a final pipeline process because there are some author preferences that match inside the guidelines and this kind of tools will alter them.

[blaggacao]

@pedrobaeza Thanks Pedro, are you aware of this feature?

# disable: pyyapf
 some code
# enable: pyyapf

Or is this not what you are referring to?

[lasley]

This pre-commit plugin framework looks nice. I'm all for it.

[pedrobaeza]

I prefer to have the pre-formatting tool as an option at the beginning of my pipeline, and not having to put comments in my code to avoid changes. This tool can be very good, and it can be used, but not imposed.

[blaggacao]

You start to convince me.. :) Anyhow, strategy of small steps is perfect... We can probably talk about silent enforcement again once we made the first step of better automation.

the pre-commit.com framework seems like a complete framework, As I have read they also support or a planning to support yapf. With it's level of maturity, I vote for it as a good tooling option. Fits also to the new strategy I read somewhere to leverage other communities.. :)

Let's move over to OCA's Consens Gathering
remember that you can express your opinion by indiferent tag, as well:

👍 for pre-commit by yelp

(For a list of avialable hooks see https://github.com/pre-commit/pre-commit-hooks)

cc/ @rvalyi @jgrandguillaume @nbessi @nhomar @max3903

[moylop260]

Fixed with the following script https://github.com/OCA/maintainer-quality-tools/blob/master/git/pre-commit