-
Notifications
You must be signed in to change notification settings - Fork 82
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
Bump version for v0.1.0 #116
Conversation
Formerly version of image-tools followed up image-spec. |
Change version to v0.1.0 |
@opencontainers/image-tools-maintainers We have got 7 LGTM votes for releasing v0.1.0. So PTAL thais PR. |
cmd/oci-image-validate/main.go
Outdated
@@ -104,7 +104,7 @@ func newValidateCmd(stdout, stderr *log.Logger) *cobra.Command { | |||
func (v *validateCmd) Run(cmd *cobra.Command, args []string) { | |||
if v.version { | |||
v.stdout.Printf("commit: %s", gitCommit) | |||
v.stdout.Printf("spec: %s", specs.Version) | |||
v.stdout.Printf("spec: %s", version.Version) |
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.
This isn't correct. We should output separate lines for what spec
versions we support, and the version of the actual tool.
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.
Also, if we need to implement better version code we really should've gotten that merged before we had a vote. The vote is "this commit should be tagged", not "this PR should be merged in whatever state it is when someone clicks the merge button".
It is separated line. But what mean actual tool version? |
The version of this tool is
Or something like that. |
I did put up this PR, and then mail for voting. Or you have other proposal to correct this? |
@xiekeyang My concern is that you're adding new code for something that should just be a |
Thanks, I should have keep spec version, and add tools version individual. Now I update commit to 704b914. The version shows as:
PTAL, thanks. |
@cyphar Maybe I should not publish the vote result in this PR, and reback the comment, which bring out confusion. But if the bump is ready? I think after it is merged, then I should tag and release v0.1.0 package? |
@xiekeyang For future reference, this is how the release process for OCI projects works:
The reason why it's done this way is so that people can be certain what they're voting on. As it stands, the vote we had references a commit which doesn't exist anymore -- which is why the commit that is being voted on should not contain code. Now, I'm of half a mind that we break out the versioning changes into a separate pull request, get that merged and then have another vote about the |
On Wed, Feb 22, 2017 at 11:26:13AM -0800, Aleksa Sarai wrote:
1. You create a PR which has the list of issues and has two commits
-- one which does the version bump and the other which switches it
back to `vX.X.X+dev`. The commits should _only_ be those changes. An
example of this is opencontainers/image-spec#566.
You may want three commits, the two you mention here and an earlier
commit updating your release notes. For example, see [1]. Having a
ChangeLog entry makes it easier for maintainers to see what they're
voting on, and makes it easy for other consumers to migate to the new
release.
There was some previous discussion about requiring ChangeLogs for OCI
releases [2], but as far as I'm aware attempts to define release
criteria have stalled out. The Core Infrastructure Initiative Best
Practices Badge requires human-oriented release notes of some form
[3], and gives projects a few ways to meet that criterion, a ChangeLog
being one of them. I expect OCI projects that don't already supply
release notes will begin adding them as they work on CII badging.
Cheers,
Trevor
[1]: https://github.com/opencontainers/runtime-spec/pull/672/commits
[2]: opencontainers/tob#15 (comment)
[3]: https://github.com/linuxfoundation/cii-best-practices-badge/blob/5a0a7d0effe1fd92f5313142f918a0de3a8d241f/doc/criteria.md#change-control
…--
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy
|
@cyphar Thank lots! I update it on 0cecf6b and 69a2401. @wking As to add changelog, I didn't find it in similar cli tools repo as runtime-tools or docker/containerd. I think it might be not a well considered opinion yet, and I'd like to listen to opinions from @opencontainers/image-tools-maintainers. |
@xiekeyang But a changelog for 500 commits where we don't have a previous version isn't really helpful IMO. Can we please get this versioned first. |
@wking We're already having enough troubles bumping a version. Please let's not pile more things on. |
@cyphar I agree the helpful changelog. But I honestly don't like to include it in this PR. Now I concern if this go forward on right direction. |
On Wed, Feb 22, 2017 at 08:12:33PM -0800, Aleksa Sarai wrote:
But a changelog for 500 commits where we don't have a previous
version isn't really helpful IMO.
I agree that you probably don't need a ChangeLog for the first
release, since nobody is upgrading from a previous release. But after
that, I think ChangeLogs are helpful. Even if there are 500 commits.
Compiling a 500 commit ChangeLog entry would not be fun, but it is
much more efficient to do it once as a community than it would be to
have every consumer dig through those 500 commits on their own to see
what changed.
And if compiling a ChangeLog is so much trouble that it makes it hard
to cut a release, then I think you need to cut more frequent releases
and/or give up on CII badging. Certainly don't stop cutting releases,
since that only makes it harder to figure out what changed.
|
LGTM, though I'm still quite frustrated that we had a vote which was not helpful (also exposing the version through an API is not necessary -- but we can fix that later). Let's just please get a version out so we can stop being blocked on this and start fixing everything else in |
[ And for some reason PullApprove is still broken and doesn't recognise I'm a maintainer... ] |
@cyphar Thanks for you explain and excuse me that I want to comply with the process of bump and vote like image-spec, but I didn't put enough attention to the potential conflict between them. About 4 steps you mentioned, they are what I think as same as in image-spec, and had tried to carry out. But for vote mail on v0.1.0 release, I think we have achieve the agreement to release tag including which commits, except bump commit. We note that vote ML (both image-spec and image-tools) announces the bump PR id, not the final commit id, (I think) that because it might need fix more. So, can we accept that after this bump PR is landed, then I release the v0.1.0 tag, without more vote? I'm not sure if I fully catch the right processing. Of course if necessary, I'd have to send a new mail, but please let me clear the needed supplement in the mail firstly, to avoid introduce more problems. To be honest, now I'm still a little confused if fixing this bump PR invalidates the previous vote result. cc @opencontainers/image-tools-maintainers |
This is not correct. ML votes are on the commit to be tagged, not the pull request. This is why the subject of all vote emails is of the format The reason it's done this way (as as opposed to voting on a PR) is because if someone rebases a PR and now the version will refer to a different codebase, it's difficult to see what the maintainers voted on. Voting on a commit ID means that everyone agrees what the history will look like and what will be tagged. Now, this means that you cannot change the contents of the voted on commit (since the commit ID will change). This is also a good thing, because it also makes sure that everyone agrees what the code will look like once it's merged. However, because you cannot change the commit, this means that you should not be making any code changes in a version bump. The reason for this should be obvious given the above comments. As an example, IMO this PR is currently invalidated because the commit that we voted on contained code changes that needed to be fixed.
Sigh. While I feel like this is a bit too much bureaucracy-for-bureaucracy's-sake, we really shouldn't get into the habit of tagging commits that we didn't agree on.
Unfortunately in my mind, it does invalidate the vote result (well unless you want to merge the old code which we voted on -- which was also incorrect). Sorry that I didn't check this PR properly before voting. Here's what we need to do in order for votes to go far more smoothly in future:
|
This looks fine. Send the email. 😸 |
Signed-off-by: xiekeyang <[email protected]>
Signed-off-by: xiekeyang <[email protected]>
Bump to v0.1.0. Vote was +7 -0 #3. Closes #116 LGTMs: @cyphar @coolljt0725 @vbatts
Tag pushed, release cut, binaries built and signed with my keys. https://github.com/opencontainers/image-tools/releases/tag/v0.1.0 |
There are merged 219 change-sets since the beginning and cleared the blockers for v0.1.0.
Signed-off-by: xiekeyang [email protected]