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

make bors ignore comments in PR template #126501

Merged
merged 1 commit into from
Jun 15, 2024
Merged

Conversation

lqd
Copy link
Member

@lqd lqd commented Jun 14, 2024

As discussed here, the existing PR template is not yet ignored by bors and the html comments show up in git history.

This PR uses markers so the html comments are deleted by the bot.

r? lcnr

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 14, 2024
@Mark-Simulacrum
Copy link
Member

@bors r+

Not 100% sure this will work, but let's try it.

@bors
Copy link
Contributor

bors commented Jun 14, 2024

📌 Commit c006748 has been approved by Mark-Simulacrum

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 Jun 14, 2024
@lqd
Copy link
Member Author

lqd commented Jun 14, 2024

bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 15, 2024
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#126354 (Use `Variance` glob imported variants everywhere)
 - rust-lang#126367 (Point out failing never obligation for `DEPENDENCY_ON_UNIT_NEVER_TYPE_FALLBACK`)
 - rust-lang#126469 (MIR Shl/Shr: the offset can be computed with rem_euclid)
 - rust-lang#126471 (Use a consistent way to filter out bounds instead of splitting it into three places)
 - rust-lang#126472 (build `libcxx-version` only when it doesn't exist)
 - rust-lang#126497 (delegation: Fix hygiene for `self`)
 - rust-lang#126501 (make bors ignore comments in PR template)
 - rust-lang#126509 (std: suggest OnceLock over Once)
 - rust-lang#126512 (Miri subtree update)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit e287a00 into rust-lang:master Jun 15, 2024
6 checks passed
@rustbot rustbot added this to the 1.81.0 milestone Jun 15, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jun 15, 2024
Rollup merge of rust-lang#126501 - lqd:fix-pr-template, r=Mark-Simulacrum

make bors ignore comments in PR template

As discussed [here](https://rust-lang.zulipchat.com/#narrow/stream/122651-general/topic/default.20PR.20description.20feedback/near/444794577), the existing PR template is not yet ignored by bors and the html comments show up in git history.

This PR uses markers so the html comments are deleted by the bot.

r? lcnr
@lqd lqd deleted the fix-pr-template branch June 15, 2024 15:38
@Kobzol
Copy link
Contributor

Kobzol commented Jun 16, 2024

I wonder what will new contributors think of this, it might confuse them:
image

Maybe we should add something like "please delete the markers at the top and bottom please" xD

@lqd
Copy link
Member Author

lqd commented Jun 17, 2024

Why? You don’t need to remove the markers if this worked.

@Kobzol
Copy link
Contributor

Kobzol commented Jun 17, 2024

Yeah, it's just that it's maybe not obvious that:

  1. The comments will disappear in the rendered text (although that can be checked with the preview)
  2. The homu comments are not actually important if you modify the text (it might seem like something important that should be kept there).

I was mostly mentioning 2). But as you said, if the markers stay there, that's fine. Unless people only replace the inner comment and put text between the markers 😀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants