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

workflows: Run urls-check in GitHub workflows #14795

Merged
merged 2 commits into from
Oct 26, 2020

Conversation

marusak
Copy link
Member

@marusak marusak commented Oct 25, 2020

Also fixing bug why no po-refresh and npm-update were triggered.

Tried it on my own fork.

@marusak marusak added the no-test For doc/workflow changes, or experiments which don't need a full CI run, label Oct 25, 2020
Copy link
Member

@martinpitt martinpitt left a comment

Choose a reason for hiding this comment

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

Thanks! One fix necessary, then we can move this. I think we should also change urls-check to not create an issue at all if the check succeeds, that's just useless noise. It's enough if it files an issue on a broken check? (That's follow-up material, of course)

.github/workflows/npm-update.yml Show resolved Hide resolved

- name: Run urls-check action
run: |
make bots
Copy link
Member

Choose a reason for hiding this comment

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

That has the same problem, though?

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah, I fix it with the other commit. Should I do this the right way in the first commit?

Copy link
Member

Choose a reason for hiding this comment

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

Ah, yes please, less confusing.

Copy link
Member Author

Choose a reason for hiding this comment

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

done

steps:
- name: Set up configuration and secrets
run: |
printf '[user]\n\tname = Cockpit Project\n\[email protected]\n' > ~/.gitconfig
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't commit anything, so we don't strictly need a git config here.

Copy link
Member Author

Choose a reason for hiding this comment

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

true, I'll drop it.

Copy link
Member Author

Choose a reason for hiding this comment

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

done

@marusak
Copy link
Member Author

marusak commented Oct 25, 2020

I think we should also change urls-check to not create an issue at all if the check succeeds, that's just useless noise. It's enough if it files an issue on a broken check?

urls-check is smart in this matter. It works like this:

  • It looks for the last issue for urls-check
  • If it is opened, it does nothing (waits until we fix it)
  • If it is closed and the last update was more than n-days ago, it runs the check
  • If the check succeeds it just comments into that closed issue that all is fine (just so we can see it ran and it bumps up the last modification [see previous point])
  • If it fails, it opens a new issue (see the second point)
  • (special case) If there is no opened nor closed issue and the check succeeded then create a new issue, comment all went fine and close it. This happens when we run this for the first time in a repository. And this is what you saw.

That would first require autogen, lets just use `tools/make-bots` instead.
Copy link
Member

@martinpitt martinpitt left a comment

Choose a reason for hiding this comment

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

Yay, thanks!

@marusak marusak merged commit 4578520 into cockpit-project:master Oct 26, 2020
marusak added a commit that referenced this pull request Oct 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-test For doc/workflow changes, or experiments which don't need a full CI run,
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants