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

feat(actions): switch to upstream CI templates #113

Merged
merged 1 commit into from
Mar 15, 2024

Conversation

laugmanuel
Copy link
Member

@laugmanuel laugmanuel commented Feb 14, 2024

  • drop tests for Foreman <= 3.5 (as it does not support the template)
  • switch generic rubocop for theforeman-rubocop
  • fix rubocop warnings

@laugmanuel laugmanuel force-pushed the feat/use-upstream-github-actions branch 3 times, most recently from 7c878b4 to 57556a4 Compare February 15, 2024 07:50
@laugmanuel
Copy link
Member Author

@archanaserver what's the recommended way of defining specific version of other plugins as dependencies?
In this case we need foreman-tasks. However, the generic action uses the latest version of the plugin which breaks on older Foreman versions (3.7 and 3.8) as the plugin defines a minimum foreman version.

@laugmanuel laugmanuel force-pushed the feat/use-upstream-github-actions branch from 57556a4 to aee308c Compare February 15, 2024 12:32
@laugmanuel
Copy link
Member Author

@archanaserver what's the recommended way of defining specific version of other plugins as dependencies? In this case we need foreman-tasks. However, the generic action uses the latest version of the plugin which breaks on older Foreman versions (3.7 and 3.8) as the plugin defines a minimum foreman version.

I guess this would require seperate branches for the targeted foreman versions. WDYT @kamils-iRonin ?

@archanaserver
Copy link

@archanaserver what's the recommended way of defining specific version of other plugins as dependencies? In this case we need foreman-tasks. However, the generic action uses the latest version of the plugin which breaks on older Foreman versions (3.7 and 3.8) as the plugin defines a minimum foreman version.

I guess this would require seperate branches for the targeted foreman versions. WDYT @kamils-iRonin ?

Thank you for raising this point about managing plugin versions. While foreman_plugin.yml itself doesn't have any mechanism for specifying exact plugin versions, I guess either we can introduce optional input (foreman_tasks_version) to accept the desired version or another way you mentioned about creating separate branches for older Foreman versions.

I believe in terms of flexibility the first one would be more flexible, and another would be easier to do it here. What are your thoughts on this, @ekohl? Do you have any specific concerns or preferences regarding these approaches?

@ekohl
Copy link
Contributor

ekohl commented Feb 19, 2024

It's a good question I didn't consider before.

Essentially you want to specify versions and let bundler handle it. We support bundler.d for plugins (https://github.com/theforeman/actions/blob/fc8754939ee6eb01be4be973fc0a588365adf8c4/.github/workflows/foreman_plugin.yml#L104-L106) and you could write up some code there. I don't think we reliably export the Foreman version in some way, but we could (by reading $foreman_checkout/VERSION and setting that as an environment variable). Then you could use that in the Gemfile to implement some logic.

Would that make sense?

@laugmanuel
Copy link
Member Author

So far we didn't need to maintain different plugin branches as the plugins were compatible with basically all recent foreman versions and we just dropped support as a whole once a breaking change was introduced.

It would be nice to keep it this way so development stays simple (without the need for backporting/cherry picking and so on).

Having the Foreman versions available would make it easier. Couldn't we just use the branch name of foreman itself to get exported? We define the versions we want to test in the Workflow anyway.

@ekohl
Copy link
Contributor

ekohl commented Feb 19, 2024

Having the Foreman versions available would make it easier. Couldn't we just use the branch name of foreman itself to get exported? We define the versions we want to test in the Workflow anyway.

I don't want that to be the interface, because we also test out PRs this way. Random example is theforeman/foreman_ansible#693

@laugmanuel
Copy link
Member Author

Having the Foreman versions available would make it easier. Couldn't we just use the branch name of foreman itself to get exported? We define the versions we want to test in the Workflow anyway.

I don't want that to be the interface, because we also test out PRs this way. Random example is theforeman/foreman_ansible#693

Understandable. What about a more open interface where the caller can pass in arbitrary variables which are exposed in the job?

@ekohl
Copy link
Contributor

ekohl commented Feb 19, 2024

I like the idea of setting arbitrary (environment) variables for specific versions. Perhaps we can track this in an issue on https://github.com/theforeman/actions and gather some broader feedback? I can imagine other plugin authors may also have feedback.

Rakefile Outdated Show resolved Hide resolved
- drop tests for Foreman <= 3.6 (as it does not support the template)
- switch generic rubocop for theforeman-rubocop
- fix rubocop warnings
@laugmanuel laugmanuel force-pushed the feat/use-upstream-github-actions branch from a5b7180 to fcfca3c Compare March 11, 2024 11:36
@laugmanuel laugmanuel marked this pull request as ready for review March 11, 2024 11:50
@laugmanuel laugmanuel mentioned this pull request Mar 11, 2024
@laugmanuel laugmanuel merged commit 3eedb81 into master Mar 15, 2024
33 checks passed
ekohl added a commit to ekohl/foreman_wreckingball that referenced this pull request Apr 9, 2024
The gemspec shouldn't contain environment variables, because it's also
used in other contexts where it could break.

In common CI we read gemfile.d/*.rb so this is the best place for
bundler overrides.

Fixes: 3eedb81 ("feat(actions): switch to upstream CI templates (dm-drogeriemarkt#113)")
ekohl added a commit to ekohl/foreman_wreckingball that referenced this pull request Apr 10, 2024
The gemspec shouldn't contain environment variables, because it's also
used in other contexts where it could break.

In common CI we read gemfile.d/*.rb so this is the best place for
bundler overrides.

Fixes: 3eedb81 ("feat(actions): switch to upstream CI templates (dm-drogeriemarkt#113)")
laugmanuel pushed a commit that referenced this pull request May 6, 2024
The gemspec shouldn't contain environment variables, because it's also
used in other contexts where it could break.

In common CI we read gemfile.d/*.rb so this is the best place for
bundler overrides.

Fixes: 3eedb81 ("feat(actions): switch to upstream CI templates (#113)")
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.

4 participants