Skip to content

[Merged by Bors] - fix(*.yml): robustify checks for line endings#16456

Closed
bryangingechen wants to merge 4 commits intomasterfrom
bgc-fix-ci-line-ending
Closed

[Merged by Bors] - fix(*.yml): robustify checks for line endings#16456
bryangingechen wants to merge 4 commits intomasterfrom
bgc-fix-ci-line-ending

Conversation

@bryangingechen
Copy link
Copy Markdown
Contributor

@bryangingechen bryangingechen commented Sep 3, 2024

Currently the bots sometimes end up failing to apply the ready to merge label due to this, see e.g. this comment and the corresponding run.

The underlying issue is probably the same as that was causing ghost maintainer merges which was fixed in #14663, so I've applied the same change to the remaining workflow files where this was a potential issue.

I found one other place in labels_from_comment.yml which was relying on line endings being \r\n so I've fixed that by using a regex that allows for both \r\n and \n.


Open in Gitpod

Specifically, check for \n instead of \r\n when scanning for "bors merge" / "bors r+" / "bors d" and split lines in labels_from_comment.yml with a regex that allows for both \r\n and \n.
@bryangingechen bryangingechen added the CI Modifies the continuous integration setup or other automation label Sep 3, 2024
@github-actions
Copy link
Copy Markdown

github-actions bot commented Sep 3, 2024

PR summary 0a903f2e9d

Import changes for modified files

No significant changes to the import graph

Import changes for all files
Files Import difference

Declarations diff

No declarations were harmed in the making of this PR! 🐙

You can run this locally as follows
## summary with just the declaration names:
./scripts/declarations_diff.sh <optional_commit>

## more verbose report:
./scripts/declarations_diff.sh long <optional_commit>

The doc-module for script/declarations_diff.sh contains some details about this script.

Copy link
Copy Markdown
Contributor

@grunweg grunweg left a comment

Choose a reason for hiding this comment

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

Thanks! I haven't checked the regex carefully, but it looks plausible: the other changes look good to me.
maintainer delegate, i.e. feel free to maintainer merge on my behalf once the dependent PR has landed.

@ghost ghost added the blocked-by-other-PR This PR depends on another PR (this label is automatically managed by a bot) label Sep 3, 2024
@kim-em
Copy link
Copy Markdown
Contributor

kim-em commented Sep 5, 2024

bors d+

@mathlib-bors
Copy link
Copy Markdown
Contributor

mathlib-bors bot commented Sep 5, 2024

✌️ bryangingechen can now approve this pull request. To approve and merge a pull request, simply reply with bors r+. More detailed instructions are available here.

@github-actions github-actions bot added the delegated This pull request has been delegated to the PR author (or occasionally another non-maintainer). label Sep 5, 2024
@mathlib4-dependent-issues-bot mathlib4-dependent-issues-bot removed the blocked-by-other-PR This PR depends on another PR (this label is automatically managed by a bot) label Sep 5, 2024
@leanprover-community leanprover-community deleted a comment Sep 5, 2024
@mathlib4-dependent-issues-bot
Copy link
Copy Markdown
Collaborator

@bryangingechen
Copy link
Copy Markdown
Contributor Author

bors r+

@github-actions github-actions bot added the ready-to-merge This PR has been sent to bors. label Sep 5, 2024
mathlib-bors bot pushed a commit that referenced this pull request Sep 5, 2024
Currently the bots sometimes end up failing to apply the `ready to merge` label due to this, see e.g. [this comment](#16452 (comment)) and [the corresponding run](https://github.com/leanprover-community/mathlib4/actions/runs/10685754793).

The underlying issue is probably the same as that was causing ghost maintainer merges which was fixed in #14663, so I've applied the same change to the remaining workflow files where this was a potential issue. 

I found one other place in labels_from_comment.yml which was relying on line endings being `\r\n` so I've fixed that by using a regex that allows for both `\r\n` and `\n`.
@mathlib-bors
Copy link
Copy Markdown
Contributor

mathlib-bors bot commented Sep 5, 2024

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title fix(*.yml): robustify checks for line endings [Merged by Bors] - fix(*.yml): robustify checks for line endings Sep 5, 2024
@mathlib-bors mathlib-bors bot closed this Sep 5, 2024
@mathlib-bors mathlib-bors bot deleted the bgc-fix-ci-line-ending branch September 5, 2024 06:10
bjoernkjoshanssen pushed a commit that referenced this pull request Sep 9, 2024
Currently the bots sometimes end up failing to apply the `ready to merge` label due to this, see e.g. [this comment](#16452 (comment)) and [the corresponding run](https://github.com/leanprover-community/mathlib4/actions/runs/10685754793).

The underlying issue is probably the same as that was causing ghost maintainer merges which was fixed in #14663, so I've applied the same change to the remaining workflow files where this was a potential issue. 

I found one other place in labels_from_comment.yml which was relying on line endings being `\r\n` so I've fixed that by using a regex that allows for both `\r\n` and `\n`.
bjoernkjoshanssen pushed a commit that referenced this pull request Sep 9, 2024
Currently the bots sometimes end up failing to apply the `ready to merge` label due to this, see e.g. [this comment](#16452 (comment)) and [the corresponding run](https://github.com/leanprover-community/mathlib4/actions/runs/10685754793).

The underlying issue is probably the same as that was causing ghost maintainer merges which was fixed in #14663, so I've applied the same change to the remaining workflow files where this was a potential issue. 

I found one other place in labels_from_comment.yml which was relying on line endings being `\r\n` so I've fixed that by using a regex that allows for both `\r\n` and `\n`.
bjoernkjoshanssen pushed a commit that referenced this pull request Sep 12, 2024
Currently the bots sometimes end up failing to apply the `ready to merge` label due to this, see e.g. [this comment](#16452 (comment)) and [the corresponding run](https://github.com/leanprover-community/mathlib4/actions/runs/10685754793).

The underlying issue is probably the same as that was causing ghost maintainer merges which was fixed in #14663, so I've applied the same change to the remaining workflow files where this was a potential issue. 

I found one other place in labels_from_comment.yml which was relying on line endings being `\r\n` so I've fixed that by using a regex that allows for both `\r\n` and `\n`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI Modifies the continuous integration setup or other automation delegated This pull request has been delegated to the PR author (or occasionally another non-maintainer). ready-to-merge This PR has been sent to bors.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants