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

Add Zeitwerk loader support #105

Merged
merged 1 commit into from
Oct 21, 2024
Merged

Add Zeitwerk loader support #105

merged 1 commit into from
Oct 21, 2024

Conversation

nadjaheitmann
Copy link
Contributor

No description provided.

@nadjaheitmann
Copy link
Contributor Author

I don't think that we will get the CI to be green as it is right now because Zeitwerk support is not backwards compatible. We can either add feature branches we can test against or drop the tests. Any other ideas? @dosas @m-bucher @maximiliankolb

Copy link
Contributor

@dosas dosas left a comment

Choose a reason for hiding this comment

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

I do not get why the tests are failing due to backwards compatibility since the failing tests are all for ruby >=3.

Dropping the tests is not an option.

@nadjaheitmann
Copy link
Contributor Author

I do not get why the tests are failing due to backwards compatibility since the failing tests are all for ruby >=3.

It is because I adjusted to work foreman_snapshot_management master branch to work Foreman develop which uses Zeitwerk. But Zeitwerk is not used for Foreman 3.11 and 3.12 . Hence, this is a breaking change.

The pipeline always checks against the master branch of this foreman_snapshot_management, no matter what happens in upstream Foreman. So the problem is that the pipeline somehow expect that the plugin is always backwards compatible which it is not. And what is okay, I think, the same happens for all upstream plugins.

@dosas
Copy link
Contributor

dosas commented Oct 14, 2024

I do not get why the tests are failing due to backwards compatibility since the failing tests are all for ruby >=3.

It is because I adjusted to work foreman_snapshot_management master branch to work Foreman develop which uses Zeitwerk. But Zeitwerk is not used for Foreman 3.11 and 3.12 . Hence, this is a breaking change.

The pipeline always checks against the master branch of this foreman_snapshot_management, no matter what happens in upstream Foreman. So the problem is that the pipeline somehow expect that the plugin is always backwards compatible which it is not. And what is okay, I think, the same happens for all upstream plugins.

In that case everything is working as expected (develop: green, 3.11 and 3.12: red)
I would opt for feature branches until 3.13 is released.

@nadjaheitmann
Copy link
Contributor Author

In that case everything is working as expected (develop: green, 3.11 and 3.12: red)

Definitely, we just need to decide how to deal with that.

I would opt for feature branches until 3.13 is released.

I would suggest to bump a version before merging this and then add a feature branch 3.0-stable .

@nadjaheitmann
Copy link
Contributor Author

In that case everything is working as expected (develop: green, 3.11 and 3.12: red)

Definitely, we just need to decide how to deal with that.

I would opt for feature branches until 3.13 is released.

I would suggest to bump a version before merging this and then add a feature branch 3.0-stable .

Actually, there are no significant new features, so we probably do not need to bump a new version. I added a new branch for foreman_snapshot_management version 3.x.x which does not run tests on the develop branch https://github.com/ATIX-AG/foreman_snapshot_management/tree/3.0-stable.

@nadjaheitmann nadjaheitmann merged commit e744580 into master Oct 21, 2024
19 of 45 checks passed
@nadjaheitmann nadjaheitmann deleted the zeitwerk_support branch October 21, 2024 07:56
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.

2 participants