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

Write documentation for each CI workflow #13778

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

BD103
Copy link
Contributor

@BD103 BD103 commented Dec 4, 2024

While I was working on #13033, I noticed that the workflows in CI are a bit difficult to understand for beginners. To amend this, I added short descriptions to the beginning of all actions in .github/workflows summarizing what they do.

r? @blyxyas

(I'm requesting your review because it's related to #13033, and I also don't know anyone else who's familiar with the CI in this project. Please point me in the right direction if there's someone better suited for this PR!)

changelog: none

@rustbot
Copy link
Collaborator

rustbot commented Dec 4, 2024

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @blyxyas (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

  • @rustbot author: the review is finished, PR author should check the comments and take action accordingly
  • @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Dec 4, 2024
Copy link
Member

@blyxyas blyxyas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the documentation changes! Just a couple nits and this should be merged. ❤️

Comment on lines +1 to +2
# This action runs tests specific to `clippy_dev`, accessing through the `cargo dev` command. It
# both checks the project as a whole and tests `clippy_dev`'s commands specifically.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be great if we specified which steps are taking for checking the project as a whole.

Suggested change
# This action runs tests specific to `clippy_dev`, accessing through the `cargo dev` command. It
# both checks the project as a whole and tests `clippy_dev`'s commands specifically.
# This action runs tests specific to `clippy_dev`, accessing through the `cargo dev` command. It
# both checks the project as a whole (formatting & updating the lints) and `clippy_dev`'s commands specifically.

@@ -1,3 +1,4 @@
# This action runs Clippy's test suite for all of its crates.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# This action runs Clippy's test suite for all of its crates.
# This action runs Clippy's test suite on the pull request patch.
# Making sure that a pull request doesn't break anything.

@blyxyas
Copy link
Member

blyxyas commented Dec 4, 2024

Usually it's @flip1995 the go-to person for CI changes, so he could give better advice than me, but let me know if you're stuck at any point in the process!

Copy link
Member

@flip1995 flip1995 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I would prefer to write documentation about the CI setup in the book. This is also one of my forever TODOs...

Maybe this gives me the necessary motivation to do this over the weekend. If not, we can merge this.

@BD103
Copy link
Contributor Author

BD103 commented Dec 8, 2024

@flip1995 would you like me to move the notes into the Clippy book instead? If you tell me where you want it, I can easily move it over. :)

@flip1995
Copy link
Member

flip1995 commented Dec 9, 2024

Nah the notes are not extensive enough to paint the full picture of how our CI setup works and why each workflow exists. I really need to write down the status quo of CI. I had to take a break from my computer this weekend though...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants