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 linter job to the ci workflow #8230

Closed
wants to merge 2 commits into from
Closed

Conversation

mmynk
Copy link
Contributor

@mmynk mmynk commented Apr 19, 2024

  • Adds a job which runs cargo fmt --check on the repo.
  • Makes rustfmt happy.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Apr 19, 2024
@mmynk mmynk force-pushed the fmt branch 2 times, most recently from 5a46756 to 4dfab9a Compare April 19, 2024 14:30
@mmynk
Copy link
Contributor Author

mmynk commented Apr 19, 2024

@brianc118 The linter fails only once the PR is imported internally. I thought it'd make it easier to fix issues faster if we added a rustfmt check to the workflow.

@brianc118
Copy link
Contributor

Thanks! Actually we had a rustfmt CI workflow at some point that got removed in f62afcd.

Unfortunately this has been a pain point for a while. I think internally we use rustfmt 2.0 with some unstable features, and there's no way for our project to opt out of it. So the changes here will probably not pass the internal linter :/

I'm still not sure what the best solution is. I can follow up and see if we can remove the "internal linter" signal to avoid confusion. When merging, we can always do the formatting on our end- it's not a big deal.

@mmynk
Copy link
Contributor Author

mmynk commented Apr 19, 2024

Oh okay, makes sense. Could we not use the same (or similar) config for the linter here?
Regardless, we do not need it for now as you said the formatting can be done on your end before merging.

Thanks for clarifying!

@mmynk mmynk closed this Apr 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants