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

Fix dash compatibility check #5063

Merged
merged 3 commits into from
Jul 2, 2020
Merged

Fix dash compatibility check #5063

merged 3 commits into from
Jul 2, 2020

Conversation

nitinjainsj
Copy link
Contributor

No description provided.

@netlify
Copy link

netlify bot commented Jul 1, 2020

Your website preview is ready! Hooray! 🎉

Built with commit 3f3bef6

https://deploy-preview-5063--pachyderm-docs.netlify.app

@@ -1319,7 +1319,7 @@ func getDefaultOrLatestDashImage(dashImage string, dryRun bool) string {
return dashImage
}
dashImage = defaultDashImage
compatibleDashVersionsURL := fmt.Sprintf("https://raw.githubusercontent.com/pachyderm/pachyderm/master/etc/compatibility/%v", version)
Copy link
Contributor

@Tryneus Tryneus Jul 1, 2020

Choose a reason for hiding this comment

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

I don't think we typically generate dash compatibility files for custom releases, so the first version arg should be the version without VersionAdditional if (and only if) VersionAdditional looks like a git commit hash.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Technically we are creating the compatibility file so we should commit it. But the instructions don't call for it, so we have not been doing it for a long time.

I'll make this change.


```
> git commit -a -m"Regenerate golden deployment manifests for $(pachctl version --client-only)"
> git push origin master
```

10) Update the
Copy link
Contributor

@Tryneus Tryneus Jul 1, 2020

Choose a reason for hiding this comment

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

If we're reconsidering the order of these steps - I'd propose we move the changelog up above the make point-release step. Obviously we can't edit the release notes at that point, but it's rather nice to have the CHANGELOG.md contain the relevant data under the release's git tag.

- `src/client/version/client.go` version values
- for a major release increment the MajorVersion and set the MinorVersion and MicroVersion to 0 --> eg. 2.0.0
- for a minor release leave the MajorVersion unchanged, increment the MinorVersion, and set the MicroVersion to 0 --> eg. 1.10.0
- for a patch release leave the MajorVersion and MinorVersion unchanged and increment the MicorVersion --> eg. 1.9.8
Copy link
Contributor

Choose a reason for hiding this comment

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

typo here

Suggested change
- for a patch release leave the MajorVersion and MinorVersion unchanged and increment the MicorVersion --> eg. 1.9.8
- for a patch release, leave the MajorVersion and MinorVersion unchanged and increment the MicroVersion --> eg. 1.9.8

3) Update client version. Commit these changes locally. You will push to GitHub in the next step.
- [Required for Major, Minor, and Patch releases]
- `src/client/version/client.go` version values
- for a major release increment the MajorVersion and set the MinorVersion and MicroVersion to 0 --> eg. 2.0.0
Copy link
Contributor

Choose a reason for hiding this comment

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

punctuation nit

Suggested change
- for a major release increment the MajorVersion and set the MinorVersion and MicroVersion to 0 --> eg. 2.0.0
- for a major release, increment the MajorVersion and set the MinorVersion and MicroVersion to 0 --> eg. 2.0.0

- [Required for Major, Minor, and Patch releases]
- `src/client/version/client.go` version values
- for a major release increment the MajorVersion and set the MinorVersion and MicroVersion to 0 --> eg. 2.0.0
- for a minor release leave the MajorVersion unchanged, increment the MinorVersion, and set the MicroVersion to 0 --> eg. 1.10.0
Copy link
Contributor

Choose a reason for hiding this comment

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

punctuation nit

Suggested change
- for a minor release leave the MajorVersion unchanged, increment the MinorVersion, and set the MicroVersion to 0 --> eg. 1.10.0
- for a minor release, leave the MajorVersion unchanged, increment the MinorVersion, and set the MicroVersion to 0 --> eg. 1.10.0

… a release. Also fix handling of custom releases in checking for dash compatibility
@nitinjainsj nitinjainsj requested a review from msteffen July 1, 2020 21:29
@nitinjainsj
Copy link
Contributor Author

@msteffen I made changes to the release process. Most of it is moving all the commits we do as part of the release process before the git tag is created. You may want to take a quick pass at it as well.

Copy link
Contributor

@Tryneus Tryneus left a comment

Choose a reason for hiding this comment

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

LGTM

@nitinjainsj nitinjainsj merged commit 2071a2d into master Jul 2, 2020
Copy link
Contributor

@msteffen msteffen left a comment

Choose a reason for hiding this comment

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

One minor suggestion but overall LGTM

etc/build/dash_compatibility.sh Show resolved Hide resolved
npepin-hub pushed a commit that referenced this pull request Nov 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants