-
Notifications
You must be signed in to change notification settings - Fork 23
UpdateCLI scripts for versions.yml #1602
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
base: main
Are you sure you want to change the base?
Conversation
For stack version we can use https://artifacts.elastic.co/releases/stack.json |
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.
Early review, need to look it at again in my laptop
…tic/docs-builder into theletterf-add-updatecli-automation
@reakaleek Switched to the |
You should be able to run something like But there might be some prequisites to make it work. I haven't worked with updatecli locally in a while. However, I remember you need to provide all those |
@reakaleek If the syntax and content looks legit (are we covering all that need to be updated in |
SGTM |
+CC @colleenmcginnis |
$ gh pr checkout 1602
$ GITHUB_TOKEN=$(gh auth token) \
GITHUB_ACTOR=v1v \
BRANCH_NAME=main \
updatecli diff \
--config updatecli/updatecli.d/versions.yml \
--values updatecli/values.d/scm.yml --debug It failed with:
Likely, I don't have access to some of those GitHub repositories; hence this implementation requires a few things:
|
@v1v Removed ECE following advice from @florent-leborgne and @shainaraskas. |
@v1v @reakaleek The script now extracts the |
do you know if https://github.com/elastic/docs-builder/pull/1602/files#diff-1f9aac33c46980c8e7615cefd22096957b1dfa1d377ebb8eef03f103d1953b6dR38-R40 is ordered by semver? Just wanna be sure it's not the first match. |
@v1v I think so? It's the same tactic we use for the Collector to avoid catching 8.x versions. |
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.
Is this exhaustive for all versions.yml?
Will it open a single PR or multiple?
One single PR |
@Mpdreamz only version we're missing is ECE, as explained above, I think, but I'd love it if @shainaraskas could validate it. |
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.
it would nice to see an example output of this by manually downgrading the version #s in versions.yml and then running the workflow.
does this open PRs? is there boilerplate PR content?
as far as I see, docs eng are the codeowners for the config file ... probably not something to change right this second if the decoupling work is still in flight, but the people who have the context to block/approve this change might not be correctly positioned to do so right now
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.
it would nice to see an example output of this by manually downgrading the version #s in versions.yml and then running the workflow.
does this open PRs? is there boilerplate PR content?
as far as I see, docs eng are the codeowners for the config file ... probably not something to change right this second if the decoupling work is still in flight, but the people who have the context to block/approve this change might not be correctly positioned to do so right now
@shainaraskas Example PR: elastic/opentelemetry#330 |
Co-authored-by: shainaraskas <[email protected]>
@v1v @reakaleek Seems like we're ready for primetime? Let's merge once Victor confirms everything is in place on the Robots' side. :) |
There are a couple of pending tasks:
|
Fixes #1498
+CC @reakaleek @bmorelli25
@v1v I've adapted the work you did for
opentelemetry
, would this be sufficient? Do we need different tokens, etc.?