search

facebookincubator / below

A time traveling resource monitor for modern Linux systems
Apache License 2.0
1.04k stars 60 forks source link

Add a linter job to the ci workflow #8230

Closed mmynk closed 5 months ago

mmynk commented 5 months ago
mmynk commented 5 months ago

@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 commented 5 months ago

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

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 commented 5 months ago

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!