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

ci: Avoiding linting CONTRIBUTORS.yml #3705

Merged
merged 2 commits into from
Dec 19, 2023

Conversation

polarathene
Copy link
Member

@polarathene polarathene commented Dec 17, 2023

Description

Disregard different feature

NOTE:

  • This change technically won't have an effect, as the linting workflow AFAIK is treated as required (mandatory), which according to docs will disregard paths / paths-ignore rules.
  • Additionally the GH Actions docs for Required Workflows now states this feature is being deprecated, a similar feature is available but is premium (paid); however the OSS tier we're on may be eligible for that? 🤷‍♂️

image

The linked blogpost is from Aug 2023, announcing deprecation and shutdown of the feature in October. I could be mistaken if this was the feature used with the linting workflow, I just recall it being managed as a required check status which required authorization to modify the organization settings which @georglauterbach has permissions for.

image


EDIT: Seems like I mixed up a different feature, and this is just the required status check, so we can probably rely on this PR to skip it for the file, which will treat it as successful 👍

image

The file is managed by the `contributors.yml` workflow, no need for linting to be triggered on PRs for that change.
@polarathene polarathene added area/ci kind/improvement Improve an existing feature, configuration file or the documentation labels Dec 17, 2023
@polarathene polarathene added this to the v13.1.0 milestone Dec 17, 2023
@polarathene polarathene self-assigned this Dec 17, 2023
@georglauterbach
Copy link
Member

So what is achieved by this PR? This PR does not seem to change the status quo at all (at least to me).

@polarathene
Copy link
Member Author

So what is achieved by this PR?

See the first link, there is a lint workflow that wasn't triggered, blocking @casperklein and myself from approving + merging the PR.

The lint workflow isn't relevant to that PR type, so the idea here was to have it ignored which should be an automatic pass (skip) avoiding the concern?

If you disagree that's fine, it's an issue that happens a few times a year and can alternatively be solved by waiting for master to be updated with something else to update the PR branch and that'll trigger the workflow (which we can't do ourselves presently when this bug happens).

@georglauterbach
Copy link
Member

The lint workflow isn't relevant to that PR type, so the idea here was to have it ignored which should be an automatic pass (skip) avoiding the concern?

Is this really the case? I mean, is ignoring it an automatic pass?

The issue at hand is that one workflow can not trigger workflows by opening a new PR. But because the linting workflow is required for all PRs, we end up in the situation we're in as of now.

It definitely does not hurt to merge this PR, maybe it even solves our problem. We'll see; I'll have a look at the next PR for contributors.

@polarathene
Copy link
Member Author

Is this really the case? I mean, is ignoring it an automatic pass?

The docs say that, see that last screenshot (with the purple highlight) I shared in the PR description.

The issue at hand is that one workflow can not trigger workflows by opening a new PR. But because the linting workflow is required for all PRs, we end up in the situation we're in as of now.

Ah, that makes sense, thanks :)

@polarathene polarathene merged commit ca2c53d into master Dec 19, 2023
1 check passed
@polarathene polarathene deleted the ci/lint-ignore-contributors-workflow branch December 19, 2023 01:41
reneploetz pushed a commit to reneploetz/docker-mailserver that referenced this pull request Dec 20, 2023
The file is managed by the `contributors.yml` workflow, no need for linting to be triggered on PRs for that change.

This should ideally skip the required check status for the lint workflow which cannot trigger implicitly for automated PRs. If this doesn't work the change should be reverted.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ci kind/improvement Improve an existing feature, configuration file or the documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants