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

Use clang-format to format API code #80

Open
AndrewScheidecker opened this issue Jun 19, 2019 · 5 comments
Open

Use clang-format to format API code #80

AndrewScheidecker opened this issue Jun 19, 2019 · 5 comments

Comments

@AndrewScheidecker
Copy link
Contributor

This would make it much easier for collaborators to ensure their changes have formatting that is consistent with the rest of the repo.

I'm locally using a .clang-format that just contains BasedOnStyle: Chromium, which is reasonably close to the existing code, but not perfect. Ideally @rossberg could add a .clang-format based on his preferences, and apply it to the existing code.

@rossberg
Copy link
Member

Yeah, I understand the suggestion, but I'm sorry to say that my prior exposure has elicited an allergy to clang-format. Symptoms are cringing pain and curling toe nails when exposed to algorithmic stupor steam-rolling over aesthetics. :)

Is the amount of code contributions to expect for this repo large enough that formatting would likely be a problem?

@AndrewScheidecker
Copy link
Contributor Author

Yeah, I understand the suggestion, but I'm sorry to say that my prior exposure has elicited an allergy to clang-format. Symptoms are cringing pain and curling toe nails when exposed to algorithmic stupor steam-rolling over aesthetics. :)

I understand your feeling, and after using clang-format for a while on WAVM, am still disappointed with the way it formats certain things. But it's also liberating to accept the automatic formatting and focus on more important issues.

Is the amount of code contributions to expect for this repo large enough that formatting would likely be a problem?

I don't know, but this repo will need to continue to evolve with WebAssembly.

@dschuff
Copy link
Member

dschuff commented Jun 19, 2019

I understand your feeling, and after using clang-format for a while on WAVM, am still disappointed with the way it formats certain things. But it's also liberating to accept the automatic formatting and focus on more important issues.

Big +1 to this.

Is the amount of code contributions to expect for this repo large enough that formatting would likely be a problem?

For me this is the only thing that matters; making collaboration easier is much more important than any one person's sense of aesthetics. Obviously, whether formatting is a "problem" could also be subjective; but the cost is small, and adding formatting early is easier than doing it later (e.g. in response to disagreement) when there's a lot of existing code.

@sbc100
Copy link
Member

sbc100 commented Oct 31, 2019

Could you upload a PR with the changes that result for formatting with BasedOnStyle: Chromium so we can see how intrusive it would be?

@AndrewScheidecker
Copy link
Contributor Author

Could you upload a PR with the changes that result for formatting with BasedOnStyle: Chromium so we can see how intrusive it would be?

@sbc100 See #115.

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

No branches or pull requests

4 participants