-
Notifications
You must be signed in to change notification settings - Fork 13
Add support for per-module versions to core #247
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
Open
mharriger
wants to merge
2
commits into
git-commit-id:master
Choose a base branch
from
mharriger:per-module-versions
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First thanks for your contribution, maybe it is due to the fact that I never fully understood this "per module" request, but could you outline why the native git does not support this feature (maybe by explaining what this "per module" even means?).
I personally would kinda dislike the fact that certain features of the plugin are not supported by all providers (jgit/git)....
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for considering this PR! Sorry for taking a while to get back to you, gmail sent the GitHub notification to the wrong folder.
There is no reason that I know of that native git couldn't support this. I guess that error message should have indicated it is not implemented (yet), not that it is not supported by the provider. I mostly just ported an existing PR to the current codebase, and that PR only implemented this functionality for JGit. I could look at adding it to the native git provider, if that's what is needed to get this PR done.
As to the "why" for this change:
Our project is structured like this (with many more modules in practice):
Both the POM inheritance and the filesystem layout follow this hierarchy.
When building a module (e.g. ModuleC2), I want to determine the last commit ID that affected its folder—similar to running:
git log .\ModuleC\ModuleC2
I can then attach the git.properties file as an artifact when deploying the module. Later, when building the full application, I can aggregate the git.properties files and publish a document with the last commit ID to modify each module. This makes it easy to identify which modules changed between two releases.
This process is especially valuable because the software is subject to strict certification requirements. If we can show that certain modules have not changed between versions, we can significantly reduce the amount of documentation required for certification.