-
Notifications
You must be signed in to change notification settings - Fork 25
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 implementation to get component names from Input manifest #393
Add implementation to get component names from Input manifest #393
Conversation
Signed-off-by: Divya Madala <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #393 +/- ##
============================================
+ Coverage 86.36% 87.01% +0.64%
- Complexity 29 31 +2
============================================
Files 86 86
Lines 220 231 +11
Branches 12 12
============================================
+ Hits 190 201 +11
Misses 22 22
Partials 8 8 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Divya Madala <[email protected]>
src/jenkins/InputManifest.groovy
Outdated
class Component implements Serializable { | ||
String name | ||
String repository | ||
|
||
Component(Map data) { | ||
this.name = data.name | ||
this.repository = data.repository | ||
} | ||
|
||
} | ||
|
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.
Any reason not implementation the entire map like here?
opensearch-build-libraries/src/jenkins/BundleManifest.groovy
Lines 54 to 72 in 14bc093
class Component implements Serializable { | |
String name | |
String version | |
String ref | |
String commit_id | |
String repository | |
String location | |
Component(Map data) { | |
this.name = data.name | |
this.version = data.version | |
this.ref = data.ref | |
this.commit_id = data.commit_id | |
this.repository = data.repository | |
this.location = data.location | |
} | |
} | |
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.
Currently Components
Property of input Manifest is not used any where, so I have only added arguments which are used by the release-branch creation job
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.
I do not agree with that approach.
If input manifest have those properties that you can use, just include them.
Bundle/Build manifest already have those why does input manifest be any different here? Unless it does not support those properties which is another case.
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.
Sure I can do that. Thanks Peter
Signed-off-by: Divya Madala <[email protected]>
Signed-off-by: Divya Madala <[email protected]> (cherry picked from commit 0721e85) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Description
Get component names from input manifest
Issues Related
opensearch-project/opensearch-build#4005
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.