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

Github: Enable dependabot for workflow dependencies #2778

Merged
merged 1 commit into from
Aug 22, 2022

Conversation

hoffie
Copy link
Member

@hoffie hoffie commented Aug 12, 2022

Short description of changes

This PR enables Github's dependabot for Github Action Workflows.
Example PRs:

CHANGELOG: Internal: Enabled automated dependency updates via dependabot and custom automation

Context: Fixes an issue?

Related: #2346

Does this change need documentation? What needs to be documented and how?

No, the PRs are self-documented.

Status of this Pull Request

Ready for review.

What is missing until this pull request can be merged?

Checklist

  • I've verified that this Pull Request follows the general code principles
  • I tested my code and it does what I want
  • My code follows the style guide
  • I waited some time after this Pull Request was opened and all GitHub checks completed without errors.
  • I've filled all the content above

@hoffie hoffie added this to the Release 3.9.1 milestone Aug 12, 2022
@hoffie hoffie added this to Triage in Tracking (old) via automation Aug 12, 2022
@hoffie hoffie moved this from Triage to Waiting on Team in Tracking (old) Aug 12, 2022
@ann0see
Copy link
Member

ann0see commented Aug 12, 2022

Thanks. There doesn't seem to be much with it?

@ann0see
Copy link
Member

ann0see commented Aug 12, 2022

The strange thing is that the CI failed on your PRs...

Edit: you probably stopped it.

@ann0see
Copy link
Member

ann0see commented Aug 12, 2022

Don't you think this PR should be documented as Internal in the changelog?

@hoffie
Copy link
Member Author

hoffie commented Aug 12, 2022

Thanks. There doesn't seem to be much with it?

Well, I've only enabled it for github-actions for now as I don't see any other matches. It seem to be really targeted at certain build environments (npm, go, ...) which perform dependency management. For github-actions, it does what it should. The config is really basic and I think that's fine.

Edit: you probably stopped it.

Yes. I've run lots of CI tests today and had to stop all those autobuild PRs at some point in order to get free slots for further tests. :)

Don't you think this PR should be documented as Internal in the changelog?

Not sure. I think we don't have a guideline regarding that yet. This PR does not change the released artifacts in any way, so I thought I'd skip the CHANGELOG.

@ann0see
Copy link
Member

ann0see commented Aug 16, 2022

I'd still like to have a changelog entry (maybe for all of these PRs squashed): Internal: Enabled automated dependency updates via dependabot and custom script

@hoffie
Copy link
Member Author

hoffie commented Aug 19, 2022

I've added an identical changelog entry for this PR and #2777.
I've opened #2788 for the discussion about CHANGELOG-worthiness.

@hoffie hoffie requested a review from ann0see August 20, 2022 08:26
Copy link
Member

@ann0see ann0see left a comment

Choose a reason for hiding this comment

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

I mean, I can’t say much here but thanks!

Tracking (old) automation moved this from Waiting on Team to In Progress Aug 22, 2022
@hoffie hoffie merged commit 1b8f486 into jamulussoftware:master Aug 22, 2022
Tracking (old) automation moved this from In Progress to Done Aug 22, 2022
schedule:
interval: weekly
commit-message:
prefix: "CI"
Copy link
Member

Choose a reason for hiding this comment

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

After the first PR was opened, I think we should be consistent for all the automated stuff. Either Build: Dependencies: CI: as prefix for all automated PRs.

Copy link
Member Author

Choose a reason for hiding this comment

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

My thinking was that dependabot touches CI-only stuff (i.e. workflow dependencies) while the other job touches things which are related to our actual build logic. Therefore, I had chosen different prefixes.

If we want to have a single prefix for all, I'd go with Build:. aqt, Qt, etc. do run in CI, but not solely there, so that would sound wrong.

Copy link
Member

@ann0see ann0see Aug 22, 2022

Choose a reason for hiding this comment

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

I think the real problem is that we haven't agreed on a standard (i.e. when to use CI:, Build:, Autobuild, ...) Probably agreeing gives benefit but also more maintenance/thinking on our side. I'd like to have not too many different prefixes.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Agreed. I don't really care if something is "CI" or "Build" or "Autobuild". If the build breaks, the build breaks. I'm certain (almost) no client or server user cares, either! 😄 So the CHANGELOG entries really can be grouped, so that most people can just skip the lot when they hit them.

If I had to pick, I'd go for "Build" because it covers most ground.

Copy link
Member Author

Choose a reason for hiding this comment

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

Change to Build added here: #2803.
I'll manually edit the titles (which are used when no CHANGELOG line is found) of the previous PRs.

hoffie added a commit to hoffie/jamulus that referenced this pull request Aug 23, 2022
@hoffie hoffie deleted the enable-dependabot-github branch August 25, 2022 19:48
ann0see pushed a commit to ann0see/jamulus that referenced this pull request Nov 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants