-
-
Notifications
You must be signed in to change notification settings - Fork 452
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
PROPOSAL: automate linting pre-commit or server hooks approach #145
Comments
I don't know yapf, but AFAICS it's a code formatting tool. IMO formatting tools usage is a personal decision, and OCA should not "adopt" any tools. 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 |
@dreispt Thanks for your reply and opinion! I personally stumbled over yapf, because it adopts the magnificent golang's 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. |
In my example, I use autopep8 in eclipse, so I configured to format automatically on save. |
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. |
I think "Preparing your dev environment" is a nice to have documentation. There will be the best place to put these kind of tool. |
The Maintainer Tools wiki sounds like the perfect place for that. |
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 👍 😄 |
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. |
@lasley thanks for you comment! My opinion is, that this is mainly a question of habit. Have you heard about 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.
I think this is acceptable for the promised benefits, don't you think? |
@blaggacao - I do agree that 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 😄 |
Absolutely, there are sure risks, but I think the apple is feisty and red... 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 |
cc/ @pedrobaeza @moylop260 because of your work on https://github.com/OCA/maintainer-quality-tools/tree/master/git and OCA/maintainer-quality-tools#242
You are the experts in this field... 😄 |
@lasley Something that caters to your objections and kind of leverages yelp community, which could be integrated: |
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. |
@pedrobaeza Thanks Pedro, are you aware of this feature?
Or is this not what you are referring to? |
This pre-commit plugin framework looks nice. I'm all for it. |
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. |
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 👍 for pre-commit by yelp (For a list of avialable hooks see https://github.com/pre-commit/pre-commit-hooks) |
Fixed with the following script https://github.com/OCA/maintainer-quality-tools/blob/master/git/pre-commit |
UPDATE: If interested, please help with consens gathering at the end of this thread...
With the aim of rapid adoption, I want to invite to an efficient discussion about the following question:
🌟 🌟 🌟
In favor of gtd effectiveness, I rather explicitly ask for closing, if rapid implementation chances are low.
The text was updated successfully, but these errors were encountered: