Closed blaggacao closed 8 years ago
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?
@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... :smile:
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. But there are lot of IDE, text editor and so on, it's hard to adopt just one tool to do this.
You're right, it's the benevolent dictator's (Guido's) decision to let us suffer... So we keep suffering. :smile:
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.
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 :+1: :smile:
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 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?
@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 :smile:
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)...
cc/ @pedrobaeza @moylop260 because of your work on https://github.com/OCA/maintainer-quality-tools/tree/master/git and https://github.com/OCA/maintainer-quality-tools/pull/242
You are the experts in this field... :smile:
@lasley Something that caters to your objections and kind of leverages yelp community, which could be integrated: http://pre-commit.com/
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?
# disable: pyyapf
some code
# enable: pyyapf
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
remember that you can express your opinion by indiferent
tag, as well:
:+1: 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
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:
:star2: :star2: :star2:
In favor of gtd effectiveness, I rather explicitly ask for closing, if rapid implementation chances are low.