Skip to content

[CI] Change docker lint message#11767

Merged
leandron merged 1 commit into
apache:mainfrom
gigiblender:docker-lint-error
Jun 22, 2022
Merged

[CI] Change docker lint message#11767
leandron merged 1 commit into
apache:mainfrom
gigiblender:docker-lint-error

Conversation

@gigiblender
Copy link
Copy Markdown
Contributor

@gigiblender gigiblender commented Jun 17, 2022

This PR changes the docker lint error message.

@areusch

cc @Mousius @driazati

@github-actions
Copy link
Copy Markdown
Contributor

Built docs for commit 9fa414294066b069162c5e80d625e0fb92c84293 can be found here.

Copy link
Copy Markdown
Contributor

@leandron leandron left a comment

Choose a reason for hiding this comment

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

Hi @gigiblender - thanks for the PR. While I acknowledge you fix is very simple and shouldn't do any harm to the code base, I don't think the usage of the [skip ci] here applies within the agreed rules.

The reason is that this PR is not fixing a blocking issue in CI, which is the intention of the [skip ci] mechanism to exist. For this reason, could you please update the header of this PR and re-trigger CI, so that we can then merge it?

For reference, here is[skip ci] header description:

tvm/jenkins/README.md

Lines 76 to 84 in b9890db

## Skipping CI
For reverts and trivial forward fixes, adding `[skip ci]` to the revert's
PR title will cause CI to shortcut and only run lint. Committers should
take care that they only merge CI-skipped PRs to fix a failure on `main` and
not in cases where the submitter wants to shortcut CI to merge a change faster.
The PR title is checked when the build is first run (specifically during the lint
step, so changes after that has run do not affect CI and will require the job to
be re-triggered by another `git push`).

@leandron
Copy link
Copy Markdown
Contributor

cc @driazati

@gigiblender gigiblender changed the title [skip ci] Change docker lint message [CI] Change docker lint message Jun 20, 2022
@github-actions
Copy link
Copy Markdown
Contributor

Built docs for commit 9fa414294066b069162c5e80d625e0fb92c84293 can be found here.

@gigiblender
Copy link
Copy Markdown
Contributor Author

Hi @gigiblender - thanks for the PR. While I acknowledge you fix is very simple and shouldn't do any harm to the code base, I don't think the usage of the [skip ci] here applies within the agreed rules.

The reason is that this PR is not fixing a blocking issue in CI, which is the intention of the [skip ci] mechanism to exist. For this reason, could you please update the header of this PR and re-trigger CI, so that we can then merge it?

For reference, here is[skip ci] header description:

tvm/jenkins/README.md

Lines 76 to 84 in b9890db

## Skipping CI
For reverts and trivial forward fixes, adding `[skip ci]` to the revert's
PR title will cause CI to shortcut and only run lint. Committers should
take care that they only merge CI-skipped PRs to fix a failure on `main` and
not in cases where the submitter wants to shortcut CI to merge a change faster.
The PR title is checked when the build is first run (specifically during the lint
step, so changes after that has run do not affect CI and will require the job to
be re-triggered by another `git push`).

Apologies @leandron. Wasn't aware of that. I changed the title and reran the CI.

@areusch
Copy link
Copy Markdown
Contributor

areusch commented Jun 22, 2022

thanks @gigiblender , @leandron can you approve?

Copy link
Copy Markdown
Contributor

@leandron leandron left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@leandron leandron merged commit 51b0d8c into apache:main Jun 22, 2022
@gigiblender gigiblender deleted the docker-lint-error branch June 22, 2022 19:18
blackkker pushed a commit to blackkker/tvm that referenced this pull request Jul 7, 2022
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.

3 participants