Skip to content

Conversation

Kobzol
Copy link
Member

@Kobzol Kobzol commented Aug 7, 2023

This PR avoids creating a GHA group at the very end of a CI workflow when some failure has happened. Before, when a failure has happened, its GHA group was not closed, however the clock drift check function would create a new group, which would actually close the group containing the error log, thus making errors hidden by default, which is not ideal.

See discussion here: https://rust-lang.zulipchat.com/#narrow/stream/326414-t-infra.2Fbootstrap/topic/GHA.20groups.20being.20closed.20on.20failures

r? bootstrap

@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels Aug 7, 2023
@rust-log-analyzer

This comment has been minimized.

@Kobzol
Copy link
Member Author

Kobzol commented Aug 7, 2023

Looks like with this change, the error is now clearly visible in the CI log 🎉 (I'll leave the commit with the error here for now so that others can look at the CI log).

@oli-obk
Copy link
Contributor

oli-obk commented Aug 7, 2023

yay

@bors delegate+

r=me with the test commit removed

@bors
Copy link
Collaborator

bors commented Aug 7, 2023

✌️ @Kobzol, you can now approve this pull request!

If @oli-obk told you to "r=me" after making some further change, please make that change, then do @bors r=@oli-obk

@Kobzol Kobzol force-pushed the ci-no-group-on-error branch from 50450d9 to 8d33608 Compare August 7, 2023 08:18
@Kobzol Kobzol marked this pull request as ready for review August 7, 2023 08:21
@Kobzol
Copy link
Member Author

Kobzol commented Aug 7, 2023

I actually have r= rights, but thanks 😆 (btw, I thought that the syntax is r=login, not r=@login, I wonder it works with @, as bors claims).

@bors r=@oli-obk

@bors
Copy link
Collaborator

bors commented Aug 7, 2023

📌 Commit 8d33608 has been approved by oli-obk

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 7, 2023
@oli-obk
Copy link
Contributor

oli-obk commented Aug 7, 2023

I actually have r= rights, but thanks 😆

I was wondering about that XD oops

@albertlarsan68
Copy link
Member

I thought that the syntax is r=login, not r=@login, I wonder it works with @, as bors claims

Both work, the @ version allows to have autocomplete, and (maybe?) reduces chances of typos.

bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 7, 2023
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#113568 (Fix spurious test failure with `panic=abort`)
 - rust-lang#114196 (Bubble up nested goals from equation in `predicates_for_object_candidate`)
 - rust-lang#114485 (Add trait decls to SMIR)
 - rust-lang#114495 (Set max_atomic_width for AVR to 16)
 - rust-lang#114496 (Set max_atomic_width for sparc-unknown-linux-gnu to 32)
 - rust-lang#114510 (llvm-wrapper: adapt for LLVM API changes)
 - rust-lang#114562 (stabilize abi_thiscall)
 - rust-lang#114570 ([miri][typo] Fix a typo in a vector_block comment.)
 - rust-lang#114573 (CI: do not hide error logs in a group)

r? `@ghost`
`@rustbot` modify labels: rollup
@scottmcm
Copy link
Member

scottmcm commented Aug 7, 2023

(btw, I thought that the syntax is r=login, not r=@login, I wonder it works with @, as bors claims).

It actually works with r=fluffy-the-cat too -- AFAIK it doesn't check what you wrote at all.

(I tend to type it with the @ for autocomplete, then remove the @ to reduce the number of pings to the person.)

@bors bors merged commit d9f3d49 into rust-lang:master Aug 7, 2023
@rustbot rustbot added this to the 1.73.0 milestone Aug 7, 2023
@Kobzol Kobzol deleted the ci-no-group-on-error branch August 7, 2023 18:20
falcucci added a commit to falcucci/changelog-it that referenced this pull request Aug 7, 2023
adds the list of contributors to the template. the contributors are: enselic, ralfjung, trolldemorted, matthiaskrgr, ttsugriy, kobzol, and lnicola.

this commit updates `example.md` by adding the contributors to the list. the changes can be seen in the diff:

```diff
diff --git a/templates/example.md b/templates/example.md
index a128037..da0ecc8 100644
--- a/templates/example.md
+++ b/templates/example.md
@@ -10,3 +10,16 @@
 - [ci: do not hide error logs in a group](rust-lang/rust#114573)
 - [:arrow_up: `rust-analyzer`](rust-lang/rust#114576)
 - [rollup of 9 pull requests](rust-lang/rust#114585)
+
+### contributors
+
+- [enselic](https://github.com/enselic)
+- [enselic](https://github.com/enselic)
+- [ralfjung](https://github.com/ralfjung)
+- [trolldemorted](https://github.com/trolldemorted)
+- [matthiaskrgr](https://github.com/matthiaskrgr)
+- [matthiaskrgr](https://github.com/matthiaskrgr)
+- [ttsugriy](https://github.com/ttsugriy)
+- [kobzol](https://github.com/kobzol)
+- [lnicola](https://github.com/lnicola)
+- [matthiaskrgr](https://github.com/matthiaskrgr)
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants