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

need add ci check #274

Closed
baoyachi opened this issue Dec 11, 2020 · 2 comments
Closed

need add ci check #274

baoyachi opened this issue Dec 11, 2020 · 2 comments
Labels
build Build or CI process

Comments

@baoyachi
Copy link
Contributor

when ci running ,should add cargo fmt,cargo fix,cargo clippy check
@sagebind sagebind added the build Build or CI process label Dec 17, 2020
@sagebind
Copy link
Owner

Configuration for rustfmt added in #276 and applied in #277. I'm not a fan of failing CI due to code style since it is trivial to apply ourselves; no need to increase the barrier to entry for potential contributors if avoidable. But also running cargo fmt every other commit is also annoying for contributors since you must pull before every new push. In addition, applying rustfmt after merging just pollutes the Git history when it would it be better to be pre-formatted as part of a squash merge.

Because of these difficulties, as a compromise, I've added one manual step for running rustfmt. PR authors and repo maintainers can now mention a bot with @teto-bot rustfmt in a PR comment, and the bot will push a commit to the PR after running cargo fmt if there are any changes. We'll run with this for a while to see how it goes I think.

@sagebind
Copy link
Owner

Clippy added in #279.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
build Build or CI process
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@sagebind @baoyachi