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

Using Google Java Code Style To Reformat Code #1060

Merged
merged 17 commits into from
Aug 31, 2020

Conversation

dota17
Copy link
Contributor

@dota17 dota17 commented Aug 13, 2020

Description

The current code format is not very good, especially when reading the code online on github. I can't stand this anymore.

I chose Google Java Code Style to reformat the source code.
The reason why I chose Google's version:

  1. There is a repo for the code format profile.
  2. There are 2 predefined Checkstyle configuration definitions that ship with the Maven Checkstyle Plugin, and the Google definition is the one. We can use it to help checking the code style.

I would consider adding the google checkstyle next and optimize the code format further.

Related Issue

Fixes #1050

How Has This Been Tested?

All new and existing tests passed.

Types of changes

I choose the intellij-java-google-style.xml and import in IDEA, and then reformat code with the tools.
The main change is the source code in the src directory.

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@dota17 dota17 marked this pull request as ready for review August 14, 2020 02:13
@marci4
Copy link
Collaborator

marci4 commented Aug 15, 2020

@dota17 you think it is worth to enforce the style via github action?

Best regards,
Marcel

@dota17
Copy link
Contributor Author

dota17 commented Aug 15, 2020

IMO, it is worth.

You can refer to the Apache Projects. When people submit their code to the repository, the code reviewer will always prompt the submitter to reformat code to meet the code style.

@marci4
Copy link
Collaborator

marci4 commented Aug 15, 2020

Sounds good, can you setup a github action for that?

@dota17
Copy link
Contributor Author

dota17 commented Aug 15, 2020

Yes, I can. But I don't have the permissions to configure.

@marci4
Copy link
Collaborator

marci4 commented Aug 16, 2020

You should be able to edit the github actions in your fork and merge with this PR

@dota17
Copy link
Contributor Author

dota17 commented Aug 17, 2020

OK, I will do it.

@marci4 marci4 self-requested a review August 25, 2020 07:34
Copy link
Collaborator

@marci4 marci4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please include an action to check for the correct formatation

@dota17
Copy link
Contributor Author

dota17 commented Aug 25, 2020

Sorry, I was a little busy last week. I'll take care of it as soon as possible.

@marci4
Copy link
Collaborator

marci4 commented Aug 25, 2020

@dota17 all good. Just wanted to write down why this PR is not yet merged.
Keep in mind, you don't have to do anything and you really don't have to be sorry for not having the time.
We all do this in our free time :)

dota17 and others added 2 commits August 25, 2020 17:07
Using Maven Checkstyle Plugin and Add Github Actions to check code style
@dota17
Copy link
Contributor Author

dota17 commented Aug 25, 2020

There are some check points that we should discuss here, such as OverloadMethodsDeclarationOrder and NonEmptyAtclauseDescription. You can see them in the github actions - Java Code Style Check with Maven

If we want to ignore the check points, we can add more filters in the file - checkstyle-suppressions.xml. If we won't, just need to modify the code.

@marci4
Copy link
Collaborator

marci4 commented Aug 25, 2020

I think we can ignore OverloadMethodsDeclarationOrder and NonEmptyAtclauseDescription

update suppressions and cleanup code
@marci4 marci4 added this to the Release 1.5.2 milestone Aug 30, 2020
@marci4
Copy link
Collaborator

marci4 commented Aug 30, 2020

@dota17 ping me when you are happy with the PR

@dota17
Copy link
Contributor Author

dota17 commented Aug 31, 2020

@marci4 It is ready to be merged.

@marci4 marci4 merged commit 2a91fe2 into TooTallNate:master Aug 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

What about adding the CodeFormatterProfile.xml with the code format ?
2 participants