Skip to content

chore: Add CI check for formatting #249

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

Merged
merged 9 commits into from
Jun 22, 2021
Merged

Conversation

lavaleri
Copy link
Contributor

@lavaleri lavaleri commented May 26, 2021

Issue #, if available:

Description of changes: Add check for formatting to CI, and format all code to adhere to google-java-format.

Notes:

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Check any applicable:

  • Were any files moved? Moving files changes their URL, which breaks all hyperlinks to the files.

@seebees
Copy link
Contributor

seebees commented May 26, 2021

I vote that we only have the checks in the CI.

@lavaleri
Copy link
Contributor Author

Used jardiff to determine that there is no bytecode change between a jar produced by this branch and master.

Used diff to ensure that javadocs produced by this branch and master have no difference, when stripped of newlines and <!--Generatedbyjavadoc(11.0.10)onWedMayXXXX:XX:XXPDT2021--> comments.

@lavaleri
Copy link
Contributor Author

Reconfirmed that there is no difference in bytecode using jardiff between this branch and master.

@seebees
Copy link
Contributor

seebees commented Jun 17, 2021

@lavaleri
Copy link
Contributor Author

@seebees You can to run the format check in the same build as the compliance check?

@seebees
Copy link
Contributor

seebees commented Jun 17, 2021

That is what I was thinking on the JS side, that is what I'm doing. I put the static code analysis all in one place...

@lavaleri lavaleri changed the title chore: Add Github Actions CI check for formatting chore: Add CI check for formatting Jun 21, 2021
Copy link
Contributor

@seebees seebees left a comment

Choose a reason for hiding this comment

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

Looks good. Can you add the jardiff and diff commands you used for posterity :)

@lavaleri
Copy link
Contributor Author

To check byte code difference:

  • checkout this branch, mvn clean, mvn install
  • mv ./target/aws-encryption-sdk-java-2.3.0.jar ./after.jar
  • checkout master branch, mvn clean mvn install
  • mv ./target/aws-encryption-sdk-java-2.3.0.jar ./before.jar
  • install jardiff: https://github.com/scala/jardiff
  • jardiff before.jar after.jar
  • confirm that any output is just pom or META-INF updates

To verify doc changes:

  • checkout this branch, mvn clean, mvn javadoc:javadoc
  • mv target/site afterdocs
  • checkout master branch, mvn clean, mvn javadoc:javadoc
  • mv target/site beforedocs
  • strip newlines and <!--Generatedbyjavadoc(11.0.10)onWedMayXXXX:XX:XXPDT2021--> tags from both afterdocs and beforedocs using your favorite regex!
  • diff -r beforedocs afterdocs
  • confirm there is no output

Copy link
Contributor

@alex-chew alex-chew left a comment

Choose a reason for hiding this comment

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

LGTM

@lavaleri lavaleri merged commit 275ce52 into aws:master Jun 22, 2021
@lavaleri lavaleri deleted the add-linter branch June 22, 2021 17:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants