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

Various OtherBranchVersionExtension fixes #128

Conversation

glimmerveen
Copy link
Contributor

Fixed a number of issues related to the OtherBranchVersionExtension, found when a project using gitflow-helper-maven-plugin is loaded into IntelliJ:

  1. Added a condition to not activate the OtherBranchVersionExtension, when the project file is pom-gitflow-massaged.xml . For unclear reasons, at times, IntelliJ's Maven integration tries to interpret the massaged pom file as well
  2. When only a subtree of a multi-module project is build, the embedded Maven execution now always loads the conventional location of the project file, rather than relying on the file location already loaded. This was needed as there are cases when running in the context of IntelliJ Maven integration findTopLevelProject() produces a project referencing a massaged pom file.
  3. Only write the massaged pom file if the model contents would change. This was done as tools like IntelliJ would otherwise see continuous file changes, which are used as trigger to synchronize. The approach implemented is able to deal with a subtle pom file change since Maven 3.6.3, where the schema location was changed towards https).
  4. Only update the model and produce the pom-gitflow-massaged.xml file for projects inside the reactor. Before when executing in a subtree of a multi-module project, it would update the project model for all modules in the multi-module project. This was especially problematic in conjunction with IntelliJ's Maven integration, which processes each module individually, and would trigger the pom-gitflow-massaged.xml for each module to be updated times the number of modules in the multi-module project; very inefficient

Consequence of IntelliJ's Maven integration before this update, is that it could also result in incorrect pom-gitflow-massaged.xml which would end-up in your local repository, and could result in build failures for dependent projects.

…found when a project using gitflow-helper-maven-plugin is loaded into IntelliJ:

1) Added a condition to *not* activate the OtherBranchVersionExtension, when the project file is pom-gitflow-massaged.xml . For unclear reasons, at times, IntelliJ's Maven integration tries to interpret the massaged pom file as well
2) When only a subtree of a multi-module project is build, the embedded Maven execution now always loads the *conventional* location of the project file, rather than relying on the file location already loaded. This was needed as there are cases when running in the context of IntelliJ Maven integration findTopLevelProject() produces a project referencing a massaged pom file.
3) Only write the massaged pom file if the model contents would change. This was done as tools like IntelliJ would otherwise see continuous file changes, which are used as trigger to synchronize. The approach implemented is able to deal with a subtle pom file change since Maven 3.6.3, where the schema location was changed towards https).
4) Only update the model and produce the pom-gitflow-massaged.xml file for projects *inside* the reactor. Before when executing in a subtree of a multi-module project, it would update the project model for *all* modules in the multi-module project. This was especially problematic in conjunction with IntelliJ's Maven integration, which processes each module individually, and would trigger the pom-gitflow-massaged.xml for each module to be updated times the number of modules in the multi-module project; very inefficient

Consequence of IntelliJ's Maven integration before this update, is that it could also result in incorrect pom-gitflow-massaged.xml which would end-up in your local repository, and could result in build failures for dependent projects.
@glimmerveen
Copy link
Contributor Author

In addition to the functional changes, I took the liberty to breakup the original afterProjectsRead function, as it had gotten quite big/nested, which made it hard to read.

I also added two integration tests that cover the additional features implemented.

@glimmerveen
Copy link
Contributor Author

@bvarner did you already have to see if you agree with the proposed changes? I think with these additional changes we could consider #105 as closed.

@bvarner bvarner merged commit feff4cb into e-gineering:development Apr 17, 2023
@glimmerveen
Copy link
Contributor Author

Thank you @bvarner for merging this! I have opened an additional PR to fix certain edge cases we ran into while using this capability #134

@glimmerveen glimmerveen deleted the bugfix/concurrency-issue-otherbranch-deployment branch April 17, 2023 19:07
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