Skip to content

CI: remove branch check steps that don’t actually do anything#29198

Merged
bors-servo merged 1 commit intoservo:masterfrom
delan:ci-remove-branch-check-jobs
Jan 11, 2023
Merged

CI: remove branch check steps that don’t actually do anything#29198
bors-servo merged 1 commit intoservo:masterfrom
delan:ci-remove-branch-check-jobs

Conversation

@delan
Copy link
Member

@delan delan commented Jan 3, 2023

f165e17 (#28778) adds a step to each job of the main build that ostensibly bails out of the job, if the context branch is not auto or one of the try branches relevant for that platform. But a step that runs exit 0 doesn’t really have any effect.

Since the main workflow already has branches limited to auto and the try branches, we can safely remove those steps without worrying about the main build running on dependabot branches etc. For example:


  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix #___ (GitHub issue number if applicable)
  • There are tests for these changes OR
  • These changes do not require tests because they affect the CI configuration

@jdm
Copy link
Member

jdm commented Jan 4, 2023

Hmm, interesting. The intent behind these checks was to support things like @bors-servo try-windows, which pushes to a branch called try-windows instead of `try. It would be nice to find a solution that actually works for that.

@jdm
Copy link
Member

jdm commented Jan 4, 2023

That being said, it doesn't need to happen in this PR, and we can remove things that are not actually making any difference.
@bors-servo r+

@bors-servo
Copy link
Contributor

📌 Commit 8663757 has been approved by jdm

@delan
Copy link
Member Author

delan commented Jan 4, 2023

Thanks for the context! I have some ideas for how to solve that, but yeah, let’s do that in a second patch.

@bors-servo
Copy link
Contributor

⌛ Testing commit 8663757 with merge f317c1d...

bors-servo added a commit that referenced this pull request Jan 4, 2023
CI: remove branch check steps that don’t actually do anything

f165e17 (#28778) adds a step to each job of the main build that ostensibly bails out of the job, if the context branch is not `auto` or one of the `try` branches relevant for that platform. But a step that runs `exit 0` doesn’t really have any effect.

Since the main workflow already has `branches` limited to `auto` and the `try` branches, we can safely remove those steps without worrying about the main build running on dependabot branches etc. For example:

* [dependabot/cargo/nom-7.1.2](https://github.com/servo/servo/actions?query=branch%3Adependabot%2Fcargo%2Fnom-7.1.2) only has a run for the pull request workflow
* [this test build](https://github.com/delan/servo/actions/runs/3830596812/jobs/6518672884) shows that even if the branch check fails, the build continues

---

- [ ] ~~`./mach build -d` does not report any errors~~
- [ ] ~~`./mach test-tidy` does not report any errors~~
- [ ] ~~These changes fix #___ (GitHub issue number if applicable)~~

<!-- Either: -->
- [ ] There are tests for these changes OR
- [x] These changes do not require tests because they affect the CI configuration
@bors-servo
Copy link
Contributor

💔 Test failed - checks-github

@jdm
Copy link
Member

jdm commented Jan 5, 2023

@bors-servo retry

@bors-servo
Copy link
Contributor

⌛ Testing commit 8663757 with merge 9449153...

bors-servo added a commit that referenced this pull request Jan 5, 2023
CI: remove branch check steps that don’t actually do anything

f165e17 (#28778) adds a step to each job of the main build that ostensibly bails out of the job, if the context branch is not `auto` or one of the `try` branches relevant for that platform. But a step that runs `exit 0` doesn’t really have any effect.

Since the main workflow already has `branches` limited to `auto` and the `try` branches, we can safely remove those steps without worrying about the main build running on dependabot branches etc. For example:

* [dependabot/cargo/nom-7.1.2](https://github.com/servo/servo/actions?query=branch%3Adependabot%2Fcargo%2Fnom-7.1.2) only has a run for the pull request workflow
* [this test build](https://github.com/delan/servo/actions/runs/3830596812/jobs/6518672884) shows that even if the branch check fails, the build continues

---

- [ ] ~~`./mach build -d` does not report any errors~~
- [ ] ~~`./mach test-tidy` does not report any errors~~
- [ ] ~~These changes fix #___ (GitHub issue number if applicable)~~

<!-- Either: -->
- [ ] There are tests for these changes OR
- [x] These changes do not require tests because they affect the CI configuration
@bors-servo
Copy link
Contributor

💔 Test failed - checks-github

@delan
Copy link
Member Author

delan commented Jan 5, 2023

@bors-servo retry

@bors-servo
Copy link
Contributor

@delan: 🔑 Insufficient privileges: not in try users

@jdm
Copy link
Member

jdm commented Jan 6, 2023

I've deployed the saltfs changes; you shouldn't have further problems issuing homu commands.

@jdm
Copy link
Member

jdm commented Jan 6, 2023

@bors-servo retry

@bors-servo
Copy link
Contributor

⌛ Testing commit 8663757 with merge 0c61f42...

bors-servo added a commit that referenced this pull request Jan 6, 2023
CI: remove branch check steps that don’t actually do anything

f165e17 (#28778) adds a step to each job of the main build that ostensibly bails out of the job, if the context branch is not `auto` or one of the `try` branches relevant for that platform. But a step that runs `exit 0` doesn’t really have any effect.

Since the main workflow already has `branches` limited to `auto` and the `try` branches, we can safely remove those steps without worrying about the main build running on dependabot branches etc. For example:

* [dependabot/cargo/nom-7.1.2](https://github.com/servo/servo/actions?query=branch%3Adependabot%2Fcargo%2Fnom-7.1.2) only has a run for the pull request workflow
* [this test build](https://github.com/delan/servo/actions/runs/3830596812/jobs/6518672884) shows that even if the branch check fails, the build continues

---

- [ ] ~~`./mach build -d` does not report any errors~~
- [ ] ~~`./mach test-tidy` does not report any errors~~
- [ ] ~~These changes fix #___ (GitHub issue number if applicable)~~

<!-- Either: -->
- [ ] There are tests for these changes OR
- [x] These changes do not require tests because they affect the CI configuration
@bors-servo
Copy link
Contributor

💔 Test failed - checks-github

@delan
Copy link
Member Author

delan commented Jan 9, 2023

@bors-servo retry

@bors-servo
Copy link
Contributor

⌛ Testing commit 8663757 with merge c45a8c1...

bors-servo added a commit that referenced this pull request Jan 9, 2023
CI: remove branch check steps that don’t actually do anything

f165e17 (#28778) adds a step to each job of the main build that ostensibly bails out of the job, if the context branch is not `auto` or one of the `try` branches relevant for that platform. But a step that runs `exit 0` doesn’t really have any effect.

Since the main workflow already has `branches` limited to `auto` and the `try` branches, we can safely remove those steps without worrying about the main build running on dependabot branches etc. For example:

* [dependabot/cargo/nom-7.1.2](https://github.com/servo/servo/actions?query=branch%3Adependabot%2Fcargo%2Fnom-7.1.2) only has a run for the pull request workflow
* [this test build](https://github.com/delan/servo/actions/runs/3830596812/jobs/6518672884) shows that even if the branch check fails, the build continues

---

- [ ] ~~`./mach build -d` does not report any errors~~
- [ ] ~~`./mach test-tidy` does not report any errors~~
- [ ] ~~These changes fix #___ (GitHub issue number if applicable)~~

<!-- Either: -->
- [ ] There are tests for these changes OR
- [x] These changes do not require tests because they affect the CI configuration
@bors-servo
Copy link
Contributor

💔 Test failed - checks-github

@delan
Copy link
Member Author

delan commented Jan 10, 2023

@bors-servo retry

@bors-servo
Copy link
Contributor

⌛ Testing commit 8663757 with merge 462c105...

bors-servo added a commit that referenced this pull request Jan 10, 2023
CI: remove branch check steps that don’t actually do anything

f165e17 (#28778) adds a step to each job of the main build that ostensibly bails out of the job, if the context branch is not `auto` or one of the `try` branches relevant for that platform. But a step that runs `exit 0` doesn’t really have any effect.

Since the main workflow already has `branches` limited to `auto` and the `try` branches, we can safely remove those steps without worrying about the main build running on dependabot branches etc. For example:

* [dependabot/cargo/nom-7.1.2](https://github.com/servo/servo/actions?query=branch%3Adependabot%2Fcargo%2Fnom-7.1.2) only has a run for the pull request workflow
* [this test build](https://github.com/delan/servo/actions/runs/3830596812/jobs/6518672884) shows that even if the branch check fails, the build continues

---

- [ ] ~~`./mach build -d` does not report any errors~~
- [ ] ~~`./mach test-tidy` does not report any errors~~
- [ ] ~~These changes fix #___ (GitHub issue number if applicable)~~

<!-- Either: -->
- [ ] There are tests for these changes OR
- [x] These changes do not require tests because they affect the CI configuration
@bors-servo
Copy link
Contributor

💔 Test failed - checks-github

@delan
Copy link
Member Author

delan commented Jan 10, 2023

@bors-servo retry

@bors-servo
Copy link
Contributor

⌛ Testing commit 8663757 with merge b178ecf...

bors-servo added a commit that referenced this pull request Jan 10, 2023
CI: remove branch check steps that don’t actually do anything

f165e17 (#28778) adds a step to each job of the main build that ostensibly bails out of the job, if the context branch is not `auto` or one of the `try` branches relevant for that platform. But a step that runs `exit 0` doesn’t really have any effect.

Since the main workflow already has `branches` limited to `auto` and the `try` branches, we can safely remove those steps without worrying about the main build running on dependabot branches etc. For example:

* [dependabot/cargo/nom-7.1.2](https://github.com/servo/servo/actions?query=branch%3Adependabot%2Fcargo%2Fnom-7.1.2) only has a run for the pull request workflow
* [this test build](https://github.com/delan/servo/actions/runs/3830596812/jobs/6518672884) shows that even if the branch check fails, the build continues

---

- [ ] ~~`./mach build -d` does not report any errors~~
- [ ] ~~`./mach test-tidy` does not report any errors~~
- [ ] ~~These changes fix #___ (GitHub issue number if applicable)~~

<!-- Either: -->
- [ ] There are tests for these changes OR
- [x] These changes do not require tests because they affect the CI configuration
@bors-servo
Copy link
Contributor

💔 Test failed - checks-github

@CYBAI
Copy link
Member

CYBAI commented Jan 11, 2023

@bors-servo retry

@bors-servo
Copy link
Contributor

⌛ Testing commit 8663757 with merge c165536...

@bors-servo
Copy link
Contributor

☀️ Test successful - checks-github
Approved by: jdm
Pushing c165536 to master...

@bors-servo bors-servo merged commit c165536 into servo:master Jan 11, 2023
@delan delan deleted the ci-remove-branch-check-jobs branch January 11, 2023 09:31
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.

4 participants