Skip to content

style: fix and enforce formatting#383

Closed
danieleades wants to merge 1 commit intopop-os:masterfrom
danieleades:format
Closed

style: fix and enforce formatting#383
danieleades wants to merge 1 commit intopop-os:masterfrom
danieleades:format

Conversation

@danieleades
Copy link
Contributor

@danieleades danieleades commented Feb 7, 2026

applies the (already configured) rust formatting, as specified in the CONTRIBUTING.md guide

also adds a github workflow to check the formatting on PRs, and a dependabot config to keep the CI job up-to-date

@mmstick
Copy link
Member

mmstick commented Feb 10, 2026

No need. Any formatting should be done in an active PR to avoid unnecessary system updates and loss of QA time.

@mmstick mmstick closed this Feb 10, 2026
@danieleades
Copy link
Contributor Author

No need. Any formatting should be done in an active PR to avoid unnecessary system updates and loss of QA time.

But... It isn't getting done in active PRs, as evidenced by the formatting needing to be applied.

I would also question a QA process which is burning time on format-only changes. That sounds pointless.

And finally, the primary purpose of this PR is to introduce a CI check for formatting- which will neither be included alongside feature PRs or impact QA

@mmstick
Copy link
Member

mmstick commented Feb 10, 2026

It is, but I only format files that I have modified.

@jacobgkau
Copy link
Member

jacobgkau commented Feb 10, 2026

I think the idea with a CI check is that it would be a one-time thing to fix current formatting issues, and after that, it would just be ensuring it is actually being done in the active PRs. (In one of my recent PRs, I noticed rustfmt was suggesting a change, but I kept it as-is to better match code that was already present; this check would've forced me to run rustfmt and accept the change, which our contributing guide says I should've done.)

Now, this is still extremely low-priority and can wait until after #380, since it has no impact on the actual functionality of the code. But I don't see why it's a bad idea, if we intend to comply with the official formatter's formatting. (If we had our own style we wanted to follow instead of rustfmt's style, that would be different.)

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

Successfully merging this pull request may close these issues.

3 participants