-
Notifications
You must be signed in to change notification settings - Fork 693
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
Enforce scalafmt 2.5.2 in travis build #782
Conversation
1787175
to
75017d3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'll be honest: I really dislike code scala code formatters... though I definitely like some of these changes. If we could make it a little less opinionated about certain things I'd probably be willing to go for it. Let me look at it more closely in the next week or so.
May I ask why ? I have to admit I use it without thinking about the "should I?"
Sure, thanks ! We can probably play with the rules and remove the checks in Travis build ! |
First, I'm definitely in the minority on this kind of thing.
I've at least once had scalafmt break code (this was a few years ago at
work and they fixed it but it's not great for trust. bugs happen of
course).
But frequently I find the way it (and almost any code formatter) groups
things makes things less clear.
I'm ok with checks if they're for reasonable things or places where it
can't really change how one reads the code. Linting rather than formatting,
basically. I like there to be a bit of flexibility.
As a dumb example: I don't particularly like hard length limits on lines.
Like, most lines shouldn't go over 100 characters but sometimes (especially
with signature boilerplate) it makes things harder to read if you break
stuff up too much.
…On Fri, May 29, 2020 at 11:19 AM Louis FRULEUX ***@***.***> wrote:
i'll be honest: I really dislike code scala code formatters...
May I ask why ? I have to admit I use it without thinking about the
"should I?"
though I definitely like some of these changes. If we could make it a
little less opinionated about certain things I'd probably be willing to go
for it. Let me look at it more closely in the next week or so
Sure, thanks ! We can probably play with the rules and remove the checks
in Travis build !
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#782 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAACLING5F3E6QM5KILXJQLRT74E5ANCNFSM4NN2WTUA>
.
|
I have to admit there are still some issues sometimes. But I have not enough experience with it to have a strong opinion with Usually I use code formatters on large code base to avoid checking the linting on PR review (for instance missing spaces). Enforcing format also avoid "quick fixes" on code that does not concern the feature. For example, when i save the file, it adds automatically missing spaces on code I don't work on. Hence, it adds some noise for the review. I will remove the enforcement on Travis' side. I kinda agree about the line-length rule, it is making the code less clear at several places in this PR. Thanks for sharing your opinion ! |
For code consistency I was wondering if we could enforce the use of
scalafmt
intravis
build.The first commit is an idea of the code after formatting with
scalafmt