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.

Sign up for GitHub

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

Jump to bottom

[CI] Run clang-format as part of the analyze workflow #23298

Closed
hramos opened this issue Feb 5, 2019 · 3 comments
Closed

[CI] Run clang-format as part of the analyze workflow #23298

hramos opened this issue Feb 5, 2019 · 3 comments
Labels
Bug Flow Good first issue Interested in collaborating? Take a stab at fixing one of these issues. Help Wanted :octocat: Issues ideal for external contributors. p: Facebook Partner: Facebook Partner Resolution: Locked This issue was locked by the bot. 🔩Test Infrastructure Test infrastructure and continuous integration.

Comments

@hramos
Copy link
Contributor

hramos commented Feb 5, 2019

We use Prettier to automatically format our JavaScript code. When a PR is opened, we run Prettier as part of our CI workflow. A bot, @pull-bot, will leave a comment whenever Prettier identifies an issue with the code.

This issue is a request for help to do the same with C++ code using clang-format. You'll find a .clangformat file in both React/ and ReactCommon/. To implement this change, you will need to update .circleci/config.yml and/or scripts/circleci/analyze_code.sh and make sure Circle CI can run clang-format.
@hramos hramos added Good first issue Interested in collaborating? Take a stab at fixing one of these issues. Help Wanted :octocat: Issues ideal for external contributors. labels Feb 5, 2019
@hramos hramos added the 🔩Test Infrastructure Test infrastructure and continuous integration. label Feb 5, 2019
@hramos hramos removed the Flow label Feb 5, 2019
@gengjiawen
Copy link
Contributor

How about introduce clang-tidy too ?
cc @dulmandakh @empyrical @matthargett @shergin

@gengjiawen
Copy link
Contributor

About the implementation, not necessarily use bash, a nodejs script might do.
We can refer to electron https://github.com/electron/electron/blob/b7afec07433b79eb942ae8d6ded10c61dcdd21bb/package.json#L47.

Also, why not just use one .clang-format in project root ?

@hramos hramos removed the Bug Report label Feb 6, 2019
@cpojer cpojer mentioned this issue Mar 19, 2019
Closed
@cpojer
Copy link
Contributor

cpojer commented Mar 20, 2019

@gengjiawen made this happen, yay!

@cpojer cpojer closed this as completed Mar 20, 2019
@facebook facebook locked as resolved and limited conversation to collaborators Mar 20, 2020
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Mar 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
Bug Flow Good first issue Interested in collaborating? Take a stab at fixing one of these issues. Help Wanted :octocat: Issues ideal for external contributors. p: Facebook Partner: Facebook Partner Resolution: Locked This issue was locked by the bot. 🔩Test Infrastructure Test infrastructure and continuous integration.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@cpojer @hramos @gengjiawen @facebook-github-bot @react-native-bot