Skip to content
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

Add an auto-formatter plugin #32

Open
kentros opened this issue May 20, 2019 · 10 comments

Comments

Projects
None yet
3 participants
@kentros
Copy link
Contributor

commented May 20, 2019

It would be a good idea to enforce the code style through a tool like Checkstyle. Otherwise I would guess that the code formatting could hinder further contributions.

Originally posted by @SoylentBob in #23 (comment)

Adding checkstyle would be good. It would be even better to add a plugin that just automatically corrects the formatting for us. If we want to standardize on the google style, we can just add fmt-maven-plugin and call it done. Or alternatively, if you want to define a custom formatting convention, the best bet is probably: formatter-maven-plugin

@ibauersachs

This comment has been minimized.

Copy link
Member

commented May 20, 2019

Yes it would. However, so far the codebase isn't in a consistent style and I'm hesitant to just auto-format everything. Something should and will be done, but I'm just no sure yet how far to go.

@kentros

This comment has been minimized.

Copy link
Contributor Author

commented May 20, 2019

Checkstyle comes with support for either Google or Sun Java Styles out of the box

My preference is the google style. Oracle doesn't appear to care about updating the code convention docs from Sun anymore.

I'm hesitant to just auto-format everything

Is there a particular fear you have with an auto-formatting commit? It shouldn't affect the behavior of the code. FWIW, if the sheer volume of trivial whitespace changes this will produce is daunting, GitHub allows for ignoring whitespace changes in the review (https://github.blog/2018-05-01-ignore-white-space-in-code-review/)

@ibauersachs

This comment has been minimized.

Copy link
Member

commented May 21, 2019

Is there a particular fear you have with an auto-formatting commit?

Losing almost the entire source code history.

@kingle

This comment has been minimized.

Copy link
Collaborator

commented May 21, 2019

Losing almost the entire source code history.

😕 How does applying a formatting change lose the history?

@ibauersachs

This comment has been minimized.

Copy link
Member

commented May 21, 2019

Every single line in a blame shows the formatting commit.

@kentros

This comment has been minimized.

Copy link
Contributor Author

commented May 21, 2019

GitHub now has an improved blame that lets you see previous blames. https://help.github.com/en/articles/tracking-changes-in-a-file

@ibauersachs

This comment has been minimized.

Copy link
Member

commented Jun 1, 2019

Okay, the format of the unit tests annoys me enough to apply a formatter on the unit test files (only!). Any preferences for the format/code style?

And btw. why do you use two Github accounts? 🤔

@kingle

This comment has been minimized.

Copy link
Collaborator

commented Jun 2, 2019

I got tired of weird messages asking if I was really "Kent Rose", so I switched to use my last name. Unfortunately, didn't log out of it on all my devices. 🤦

As for style, I don't really care. What plugin are you planning on using? If using formatter-maven-plugin, whatever default it supplies is fine:
https://code.revelc.net/formatter-maven-plugin/examples.html

@ibauersachs

This comment has been minimized.

Copy link
Member

commented Jun 2, 2019

I got tired of weird messages asking if I was really "Kent Rose", so I switched to use my last name. Unfortunately, didn't log out of it on all my devices. 🤦

https://help.github.com/en/articles/changing-your-github-username?

As for style, I don't really care. What plugin are you planning on using? If using formatter-maven-plugin, whatever default it supplies is fine:
https://code.revelc.net/formatter-maven-plugin/examples.html

Just checkstyle and maybe committing the IntelliJ formatting options.

@kingle

This comment has been minimized.

Copy link
Collaborator

commented Jun 2, 2019

Sounds good

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.