-
Notifications
You must be signed in to change notification settings - Fork 272
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
Auto-generate next version manifest(s) #461
Conversation
324a8e6
to
d88a549
Compare
Codecov Report
@@ Coverage Diff @@
## main #461 +/- ##
==========================================
+ Coverage 65.18% 68.32% +3.13%
==========================================
Files 51 59 +8
Lines 1307 1487 +180
==========================================
+ Hits 852 1016 +164
- Misses 455 471 +16
Continue to review full report at Codecov.
|
cba6d14
to
d7db869
Compare
def get_root_branches(self): | ||
branches = ["main"] | ||
remote_branches = ( | ||
subprocess.check_output( |
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.
Move to GitRepository? Or some kind of git actions class
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.
Makes sense, will revisit in another PR.
run: | | ||
./bundle-workflow/manifests.sh update | ||
- name: Create Pull Request | ||
uses: peter-evans/create-pull-request@v3 |
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.
Could this pile up duplicate pull requests? Do we have any control over managing duplication?
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.
The plugin assumes a branch name (which here I didn't specify so it will just be called "patch") and if it sees a PR from a branch called "patch", it will update it instead of creating a new one. It's pretty smart like that.
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.
Awesome! Could we add a branch name that is based off of the version number? This way if there is a patch release and a minor release updates at the same time separate pull requests are created.
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.
Not so easy to know what's being added, would require returning something meaningful from the build that can be parsed by the workflow. I did update the branch name to manifests/auto
so that the auto-pr doesn't walk over other PRs.
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.
Setting branch:
didn't work on my fork with a permissions error that doesn't happen with defaults, so I undid that.
Signed-off-by: dblock <[email protected]>
Signed-off-by: dblock <[email protected]>
Signed-off-by: dblock <[email protected]>
Signed-off-by: dblock <[email protected]>
Signed-off-by: dblock <[email protected]>
Signed-off-by: dblock <[email protected]>
Signed-off-by: dblock <[email protected]>
d7db869
to
ddf08b0
Compare
Signed-off-by: dblock <[email protected]>
Signed-off-by: dblock <[email protected]>
Signed-off-by: dblock <[email protected]>
Signed-off-by: dblock <[email protected]>
Signed-off-by: dblock <[email protected]>
super().__init__("OpenSearch", repo, snapshot) | ||
|
||
@classmethod | ||
def get_root_branches(self): |
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.
We should document what it means to be a 'root branch' so that it is clear feature branches with the same versions of a 'root branch' are excluded.
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.
Will do.
from system.properties_file import PropertiesFile | ||
|
||
|
||
class ComponentOpenSearch(Component): |
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.
Nit - OpenSearchComponent & OpenSearchMinComponent?
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 like that file names sort :)
|
||
### Auto-Generate Manifests | ||
|
||
The [manifests workflow](src/manifests) reacts to version increments in OpenSearch and its components by extracting Gradle properties from project branches. When a new version is identified, it creates a new input manifest in [manifests](../manifests) and [opens a pull request](../.github/workflows/manifests.yml). |
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.
Can we specify which branches of core & components are scanned?
If I'm reading this correctly, we are only scanning branches of core (with the specific 'root' pattern) and the 'main' branch of component repos. Is that correct?
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.
Correct. We’re looking at anything that seems like a version number. It’s really not that important though, we could be examining all branches.
We should probably not actually be looking at branches like 1.0 to save time - in our branching strategy we always increment on main or 1.x first.
Signed-off-by: dblock [email protected]
Description
Automatically generate next version's manifest.
Now generates OpenSearch 1.0.1, 1.2.0 and 2.0.0 manifests.
I'm still writing more tests, but otherwise this is ready.
Issues Resolved
Closes #388.
Check List
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.