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

Add --json option to release and releases #2685

Merged
merged 2 commits into from
Oct 20, 2023

Conversation

bbugh
Copy link
Contributor

@bbugh bbugh commented Oct 17, 2023

Resolves: #2351
Change-type: minor


This adds -j/--json to the release and releases subcommands. #2351 mentions that support would be added in v14 but there's nothing yet in v17!

We needed this for automating pinning multiple testing devices based on release tags, beyond what balenaCloud allows with "pin to latest".

I followed existing commands so it should match the existing codebase, and I added spec tests for the functionality.

@ab77
Copy link
Contributor

ab77 commented Oct 17, 2023

@klutchell remind me please, for external contributions on repos utilising self-hosted runners, someone from the balena org would need to create a separate PR from @bbugh bbugh:add-releases-json branch and push/test/merge/deploy?

@klutchell
Copy link
Contributor

@ab77 I added the required label but I think this is the first time we've tested this limitation.

Now that I'm seeing it in action, a separate label for each approved sha is secure, but will clutter the repo labels.

Another approach would be a run-ci label for approval that doesn't expire or automatically gets removed by Flowzone.

Or we wait and remove the label precautions when we launch VM isolated runners.

@klutchell
Copy link
Contributor

Thanks for the PR @bbugh !

It looks like you are missing Change-type: in your commit messages. Have a look at CONTRIBUTING.md for guidance!

@ab77
Copy link
Contributor

ab77 commented Oct 17, 2023

@bbugh Thank you for contributing. In order for automatic versioning to kick in, could you please rebase to a single commit and add the change-type as the last line of the commit comment (e.g.):

blah blah

change-type: minor

@bbugh
Copy link
Contributor Author

bbugh commented Oct 17, 2023

Thanks, y'all are so fast!

@klutchell It looks like you are missing Change-type: in your commit messages. Have a look at CONTRIBUTING.md for guidance!

I can fix this for sure. I did actually read through CONTRIBUTING.md multiple times, but it's really dense! To make it easier for contributors, I think it might be helpful to be broken up a bit into sections like "preparation", "working", and "submitting a PR" or something like that, with a clearly defined acceptance checklist for submitting.

@ab77 Thank you for contributing. In order for automatic versioning to kick in, could you please rebase to a single commit and add the change-type as the last line of the commit comment (e.g.):

I can definitely do this, though I structured it this way intentionally because of my reading of the contributing guide, and just want to double check with you first. The guide appears to suggest to have multiple commits for the purpose of generating the correct CHANGELOG output:

Because of the way that the Changelog file is automatically updated from commit messages, which become the source of "what's new" for CLI end users, we advocate "meaningful commits" and user-focused commit messages. A meaningful commit is one that, in isolation, introduces a fix or feature (or part of a fix or feature) that makes sense at the Changelog level, and which leaves the CLI in a non-broken state.

Each of these commits is adding --json functionality for a different command, which in my thinking was two features. Would you verify that you want me to squash the two commits anyway?

I also see that in the changelog there's many variations of <subcommand>: <change> which would be a different format if I squash the commits. How should I format the subject if I squash them, like release, releases: Add '--json' option for JSON output?

Thanks!

@otaviojacobi
Copy link
Contributor

otaviojacobi commented Oct 17, 2023

Hello @bbugh, you can leave both commits, as long of one of them is in the format as pointed by @ab77 the CI should pickup (you can just git commit --amend the last commit and edit to be in the format

releases: Add '--json' option for JSON output

change-type: minor

Please be mindfull of the line break between the commit message and the change-type footer

@otaviojacobi
Copy link
Contributor

@bbugh Overall your changes look very good, thank you so much for the PR! I left two comments that I think would need addressing before merging + release, but except for that, thanks for the good work :)

@bbugh bbugh force-pushed the add-releases-json branch from 9220eb0 to d2f96b4 Compare October 18, 2023 14:01
@bbugh
Copy link
Contributor Author

bbugh commented Oct 18, 2023

Hello @bbugh, you can leave both commits, as long of one of them is in the format as pointed by @ab77 the CI should pickup (you can just git commit --amend the last commit and edit to be in the format

I amended with the updated commit message and force pushed, thanks!

@otaviojacobi
Copy link
Contributor

@klutchell @ab77 I think versionist should work now, is there anything else I need to setup here to have the CI running?

@klutchell
Copy link
Contributor

@otaviojacobi I added the missing label

@klutchell
Copy link
Contributor

@otaviojacobi we will need this PR merged first: #2686

Then we need to add the run-ci-<sha> label again when this PR is rebased.

Thanks for your patience on this @bbugh, we have to jump through a few (untested) hoops to allow external contributions for security reasons.

@otaviojacobi
Copy link
Contributor

Hello @bbugh just an update in here, from my side the code looks good and I will approve & merge once the CI is passing. I am investigating still on why is it breaking (I suppose it is because rebasing via github UI, but still looking into it)

docs/balena-cli.md Show resolved Hide resolved
@bbugh bbugh force-pushed the add-releases-json branch from 308bf52 to 733dab5 Compare October 20, 2023 12:28
@otaviojacobi otaviojacobi merged commit 87ba364 into balena-io:master Oct 20, 2023
@klutchell
Copy link
Contributor

Thank you for the contribution @bbugh !

@bbugh
Copy link
Contributor Author

bbugh commented Oct 20, 2023

Awesome! Thanks everybody for the help!

FYI: the two commits were not output to the changelog, the changelog doesn't list the releases command change, only release.

c785d01#diff-b9d4d95c7b8db1b90c77eb25684bdda80ca5fc14a2bf85f4a1709b5cb9c57087

balena-cli/CHANGELOG.md

Lines 7 to 11 in c785d01

## 17.2.0 - 2023-10-20
* release: Add '--json' option for JSON output [Brian Bugh]
## 17.1.7 - 2023-10-20

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for --json flag for releases
4 participants