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 a code formatter action to our repository #1664

Open
wants to merge 9 commits into
base: dev
Choose a base branch
from

Conversation

AdamBJohnsonx
Copy link
Contributor

@AdamBJohnsonx AdamBJohnsonx commented Jan 4, 2022

Add a code formatter that runs on every pr action.

This is roughly equivalent to what happens in the OneAuth/MSAL-cpp codebase. Every time you check in, we run a formatter on the code and if there is a difference it creates another commit and checks it in. You have to review these commits, because we don't trust the formatter, but with the exception of this giant review they should be small and easy to follow.

In addition to the formatter running, moving lines around broke several suppressions. Rather than move them, I just either fixed them in the case of the extra-string-creation pieces, or added a suppression comment that won't break when the format runs.

The benefits of using code formatters is that they will help with merges. If you run the same code formatter on your branch, then the merge difference will be smaller, particularly when you're messing with import statements and other silly things. This also puts us closer to having AOSP style enforcement.

If you're working on something that's spanning this change, an easy way to work around it if you don't want to just get the command line tool and run it is to just take the formatter yml file into your branch, altered to run on your branch instead of dev, check it in, pull the result, and remove the formatter file.

@AdamBJohnsonx AdamBJohnsonx added No-4j-Changelog There is no changelog entry for common4j in this PR No-Changelog This Pull-Request has no associated changelog entry. labels Jan 4, 2022
@shahzaibj
Copy link
Contributor

What's the best way to review this PR? Specific files I should be looking at?

@AdamBJohnsonx
Copy link
Contributor Author

What's the best way to review this PR? Specific files I should be looking at?

I'd generally scan it for sanity. The only change I made was in the .yml file, and the action reformatted everything else.

@AdamBJohnsonx AdamBJohnsonx changed the title Add the formatter yml Add a code formatter action to our repository Jan 4, 2022
@AdamBJohnsonx AdamBJohnsonx removed the No-4j-Changelog There is no changelog entry for common4j in this PR label Jan 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
No-Changelog This Pull-Request has no associated changelog entry.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants