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

Also run CI an the main/master branch #864

Merged
merged 2 commits into from
Jan 22, 2024
Merged

Conversation

smortex
Copy link
Member

@smortex smortex commented Jan 9, 2024

When merging a passing PR which was not on top of the last commit of the
main branch, we may break CI. This make sure a CI run is triggered when
the main branch is updated. As GitHub kindly display the status of the
last commit on a project page, this allow to quickly see if a module CI
works as intended or not.

The main branch of most of our modules is master but a few use the
more inclusive main name, so enable ci for both.

When merging a passing PR which was not on top of the last commit of the
main branch, we may break CI.  This make sure a CI run is triggered when
the main branch is updated.  As GitHub kindly display the status of the
last commit on a project page, this allow to quickly see if a module CI
works as intended or not.

The main branch of most of our modules is `master` but a few use the
more inclusive `main` name, so enable ci for both.
@bastelfreak
Copy link
Member

Can you test it on https://github.com/voxpupuli/puppet-example?

Comment on lines 11 to 12
- main
- master
Copy link
Member

Choose a reason for hiding this comment

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

AFAIK we have the remote git branch in modulesync but I'm trying to find out how to use that in a template.

Copy link
Member

Choose a reason for hiding this comment

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

I think it makes a migration in the future easier if we use both names here? Or is there a chance that a module has a different branch name other than main or master?

Copy link
Member

Choose a reason for hiding this comment

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

Turns out we don't: https://github.com/voxpupuli/modulesync/blob/ea247449927d01c8c4fc11c6fd0dbc1e531fbc32/lib/modulesync.rb#L127-L133 does provide git_base but https://github.com/voxpupuli/modulesync#configuring-modulesync-defaults does tell the user there's an option for branch. That means you could run with develop as the default branch.

I can also see a case for making it configurable. In Foreman we also have *-stable that we like to run on. So IMHO supporting overriding it in config_defaults.yaml would be nice.

@smortex
Copy link
Member Author

smortex commented Jan 9, 2024

Can you test it on voxpupuli/puppet-example?

voxpupuli/puppet-example#39

@bastelfreak
Copy link
Member

Some consumers of modulesync want to enable this for e.g. a `develop`
branch, so make it tunable and use the `main` and `master` names as
default value for this new setting which should work out-of the box for
most cases.
@smortex
Copy link
Member Author

smortex commented Jan 22, 2024

👋 I am back from vacations 🏝️

I added a config entry as suggested by @ekohl, and checked it was working as expected by running modulesync on the puppet-example module that produce no diff. Ready for a new review, thanks!

@bastelfreak bastelfreak merged commit 3ce09a3 into master Jan 22, 2024
4 checks passed
@bastelfreak bastelfreak deleted the also-run-ci-on-main-branch branch January 22, 2024 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants