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

Update create_release script #3668

Merged
merged 4 commits into from Oct 27, 2021
Merged

Conversation

antgamdia
Copy link
Contributor

Description of the change

During our recent release, we noticed the create_script wasn't working. I did not investigate the root causes but instead opted for getting rid of plenty of ad-hoc scripts as we are leveraging the new Githuib CLI.

Benefits

Plenty of curl requests are now a single gh create release command. Simpler and clearer.

Possible drawbacks

Can't think of any, as, in fact, it wasn't working recently.

Applicable issues

Additional information

Here is a demo in a private repo where I tested it worked:

releaseProcess.mp4

Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>
Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>
Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>
Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>
Copy link
Contributor

@absoludity absoludity left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the demo too!

- "*"
exclude:
authors:
- dependabot
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice


source $(dirname $0)/release_utils.sh
KUBEAPPS_REPO="kubeapps/kubeapps"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you manually change this for your demo video? Wondering if it's worth having it as ?= or even defaulting to the current repo (so it works OOTB if we move repos in the future or similar).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, I manually changed that for the video. Indeed, it is something I'd like to refactor in the rest of the scripts, but here I just followed the same approach.

Given that we may want the repo in the future, we'd like to have all these variables as CI vars, not hardcoded constants. I'd rather do it in a separate PR to also check whether it also happens in the rest of the scripts.

However, I wouldn't fall back to the current repo (even if it is the default behavior of the Github CLI if you don't pass the -R <repo> flag. Since the scripts are doing kind of important things (releasing, PRs, commits, ...) I feel more comfortable and confident if I explicitly see which repo is a command targeting to. I'd rather add a check and would return an error if undefined.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, makes sense.

@antgamdia antgamdia merged commit 8174883 into vmware-tanzu:master Oct 27, 2021
@antgamdia antgamdia deleted the releaseScript branch October 27, 2021 09:18
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.

publish CI workflow is not working
2 participants