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

Tweak GHA config #240

Merged
merged 3 commits into from
Oct 17, 2020
Merged

Tweak GHA config #240

merged 3 commits into from
Oct 17, 2020

Conversation

JohnTitor
Copy link
Member

No description provided.

@JohnTitor JohnTitor mentioned this pull request Oct 17, 2020
Comment on lines 2 to 7
on:
pull_request:
types: [opened, synchronize, reopened]
push:
branches:
- master
Copy link
Contributor

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?

Copy link
Member Author

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)

Copy link
Contributor

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.

Copy link
Member Author

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.
@JohnTitor
Copy link
Member Author

Rebased and dropped the last commit, PTAL :)

@JohnTitor
Copy link
Member Author

Oh, we've encountered another failure but I'm not sure about this, maybe caused by the mdbook version?

@ehuss
Copy link
Contributor

ehuss commented Oct 17, 2020

Oh, the link checks are failing due to the update to mdbook. To fix that, the environment variable MDBOOK_OUTPUT__HTML__INPUT_404 can be set to an empty value. It might be best to put that in the linkcheck.sh script directly. Would you be willing to make a PR to rust-lang/rust to add that? I can review/merge that.

@JohnTitor
Copy link
Member Author

Sure! Will do that soonish.

JohnTitor added a commit to JohnTitor/rust that referenced this pull request Oct 17, 2020
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.
Copy link
Contributor

@ehuss ehuss left a comment

Choose a reason for hiding this comment

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

Thanks!

@ehuss ehuss merged commit 69333ed into rust-lang:master Oct 17, 2020
@JohnTitor JohnTitor deleted the gha branch October 17, 2020 22:46
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Oct 29, 2020
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)
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.

None yet

3 participants