Skip to content
This repository has been archived by the owner on Sep 9, 2020. It is now read-only.

status: LATEST should be of the same type as VERSION #1515

Merged
merged 2 commits into from
Jan 16, 2018

Conversation

darkowlzz
Copy link
Collaborator

What does this do / why do we need it?

This change modifies the LATEST field in status from always being a
revision to the same type as of the VERSION field.

If VERSION is a semver version, the LATEST would have the latest semver
version of the project constrained by the effective constraint.
If VERSION is a branch version, the LATEST would have the latest
revision of that branch.

More precise tests can be added for this change after #1498 .

What should your reviewer look out for in this PR?

Implementation details and the change in existing tests.

Do you need help or clarification on anything?

No.

Which issue(s) does this PR fix?

None.

@darkowlzz darkowlzz added this to the v0.4.0 milestone Jan 9, 2018
@darkowlzz darkowlzz requested a review from sdboyer January 9, 2018 19:08
This change modifies the LATEST field in status from always being a
revision to the same type as of the VERSION field.

If VERSION is a semver version, the LATEST would have the latest semver
version of the project constrained by the effective constraint.
If VERSION is a branch version, the LATEST would have the latest
revision of that branch.
@darkowlzz darkowlzz force-pushed the status-latest-version-type branch from f0c97b0 to 7ed3aa2 Compare January 11, 2018 18:08
@googlebot
Copy link
Collaborator

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this State. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

// Latest should be of the same type as the Version.
if bs.Version.Type() == gps.IsSemver {
bs.Latest = v
} else {
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't account for plain versions - those will still show up with a rev. Can deal with it later, though.

Copy link
Collaborator Author

@darkowlzz darkowlzz Jan 19, 2018

Choose a reason for hiding this comment

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

But we enter this version's updatability check block only for non-rev and non-plain version. LATEST for plain version would be empty. I think that's the right behavior.

@sdboyer sdboyer merged commit 42d3398 into golang:master Jan 16, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants