-
Notifications
You must be signed in to change notification settings - Fork 278
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
Tweak GHA config #240
Tweak GHA config #240
Conversation
.github/workflows/main.yml
Outdated
on: | ||
pull_request: | ||
types: [opened, synchronize, reopened] | ||
push: | ||
branches: | ||
- master |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you explain more why these changes are made?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This has little impact but it's mainly for not triggering CI twice on PRs when someone pushes commits to this repo, not their forked repo. Sometimes folks having write access make a branch to fix a typo or any minor change and it'll trigger two jobs when they submit a PR.
Also, set PRs trigger events explicitly, in case the default to trigger events is changed (reference: https://docs.github.com/en/free-pro-team@latest/actions/reference/events-that-trigger-workflows#pull_request)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But if we need to create branches for things like beta backports (unlikely, but possible), it won't run. Also, I'm not sure it's a good idea to create random branches in this repo, since that impacts everyone. And running CI a few extra times shouldn't have a negative impact, should it?
I personally would just leave the defaults, since they just add noise and complexity. It seems unlikely that the defaults will change, and if they do it seems possible that this would have the undesired effect of missing new events that should be running.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, I'm not sure it's a good idea to create random branches in this repo, since that impacts everyone.
I totally agree with this but editing something on GitHub creates a branch on the repo (for instance, https://github.com/rust-lang/rust/tree/LeSeulArtichaut-patch-1 is accidentally created).
And running CI a few extra times shouldn't have a negative impact, should it?
Makes sense. I don't like running CI twice because I've seen it will cause spurious failure (for instance, 429 failure occurred on the rustc-dev-guide frequently and it was really noisy for us). But in this repo, CI isn't marked as required and it seems there's no spurious failure so running it multi times could be fine.
But if we need to create branches for things like beta backports (unlikely, but possible), it won't run.
This should be caught indeed, yeah.
So, I'm fine to drop that commit for now, given the above points. Thanks!
Note that the repository's default branch is now `main`, not `master`. And this should be safer to avoid breaking change.
Rebased and dropped the last commit, PTAL :) |
Oh, we've encountered another failure but I'm not sure about this, maybe caused by the mdbook version? |
Oh, the link checks are failing due to the update to mdbook. To fix that, the environment variable |
Sure! Will do that soonish. |
Set `MDBOOK_OUTPUT__HTML__INPUT_404` on linkchecker This is found in rust-lang/nomicon#240. It seems the spurious failure shows up without this flag.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Update books ## nomicon 7 commits in 6e57e64501f61873ab80cb78a07180a22751a5d6..69333eddb1de92fd17e272ce4677cc983d3bd71d 2020-09-14 11:40:23 -0400 to 2020-10-17 15:44:12 -0700 - Tweak GHA config (rust-lang/nomicon#240) - Fix link for `[T]` (rust-lang/nomicon#239) - Update casts.md (rust-lang/nomicon#232) - [WIP] Add more links (rust-lang/nomicon#180) - Data Race definition should be more precise (rust-lang/nomicon#219) - Update the diagnostic of `error[E0597]` in dropck.md (rust-lang/nomicon#157) - fix typo in Lifetimes mutable reference aliasing section (rust-lang/nomicon#225) ## reference 3 commits in 1b78182e71709169dc0f1c3acdc4541b6860e1c4..10c16caebe475d0d11bec0531b95d7697856c13c 2020-10-11 13:53:47 -0700 to 2020-10-25 20:51:26 -0700 - Add `unsafe` for `mod` and `extern`. (rust-lang/reference#898) - mention how unions interact with dropping (rust-lang/reference#897) - Add `move_ref_pattern` docs (rust-lang/reference#881) ## book 2 commits in 451a1e30f2dd137aa04e142414eafb8d05f87f84..13e1c05420bca86ecc79e4ba5b6d02de9bd53c62 2020-10-05 09:11:18 -0500 to 2020-10-20 14:57:32 -0500 - Referencing to Appendix B (rust-lang/book#2481) - Use GITHUB_PATH instead of add-path (rust-lang/book#2477) ## rust-by-example 2 commits in 152475937a8d8a1f508d8eeb57db79139bc803d9..99eafee0cb14e6ec641bf02a69d7b30f6058349a 2020-10-09 09:29:50 -0300 to 2020-10-21 14:21:55 -0300 - Formatting footer items. (rust-lang/rust-by-example#1385) - Add partial moves example for `move_ref_pattern` stabilization (rust-lang/rust-by-example#1377) ## edition-guide 3 commits in 81f16863014de60b53de401d71ff904d163ee030..7bc9b7a5e800f79df62947cb7d566fd2fbaf19fe 2020-08-27 13:56:31 -0700 to 2020-10-23 18:31:23 -0500 - A few small updates. (rust-lang/edition-guide#221) - Clarify the limitation of ? in main and tests (rust-lang/edition-guide#219) - Update deprecated GitHub Actions commands. (rust-lang/edition-guide#220)
No description provided.