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 a hook to ensure the client's version is updated before pushing a tag/release #115

Merged
merged 6 commits into from Jul 23, 2020
Merged

Conversation

MihaiBojin
Copy link
Contributor

Description

  • Versioned the user agent string

Link to any related issue(s):

Type of change:

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Required Checklist:

  • I have signed the MongoDB CLA
  • I have added tests that prove my fix is effective or that my feature works
  • I have added any necessary documentation (if appropriate)
  • I have run make fmt and formatted my code

Further comments

@MihaiBojin MihaiBojin requested a review from gssbzn July 21, 2020 14:47
mongodbatlas/version.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@gssbzn gssbzn left a comment

Choose a reason for hiding this comment

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

I have a couple of suggestions, also I'm a bit worry about the version falling out of sync again but we can follow on that with improving our release process

mongodbatlas/version.go Outdated Show resolved Hide resolved
mongodbatlas/version.go Outdated Show resolved Hide resolved
mongodbatlas/version.go Outdated Show resolved Hide resolved
mongodbatlas/version.go Outdated Show resolved Hide resolved
gssbzn
gssbzn previously approved these changes Jul 21, 2020
Copy link
Collaborator

@gssbzn gssbzn left a comment

Choose a reason for hiding this comment

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

LGTM @themantissa ready for you, it would be a good idea if we defined a release procedure to make sure we update the version on tag

@gssbzn gssbzn requested a review from themantissa July 21, 2020 17:18
themantissa
themantissa previously approved these changes Jul 21, 2020
Copy link
Collaborator

@themantissa themantissa left a comment

Choose a reason for hiding this comment

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

LGTM, @gssbzn a very good point on a documented release process. Thus far we've simply released when we needed a new version for Terraform (or other work) but with all the new dependencies I think it's a very smart suggestion to formalize this. Do you have an example we can follow?

Also adding DoU for review.

@gssbzn
Copy link
Collaborator

gssbzn commented Jul 21, 2020

in mcli we have a RELEASING.md. We also have a little bash script to automate some of the process, we could look if making a bash script to update the version and tag the release would make sense here

@themantissa
Copy link
Collaborator

@gssbzn this is an excellent example, thank you! Sounds like we'll want similar for this but perhaps figure out communication channels between teams for timing, etc.

@MihaiBojin MihaiBojin dismissed stale reviews from themantissa and gssbzn via 588ba31 July 22, 2020 10:06
run: make tools lint test

- name: Checking the version
run: make check-version
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This workflow will ensure that we get notified when a tag was pushed and the version was not updated.

@@ -13,26 +13,36 @@ link-git-hooks:
find .git/hooks -type l -exec rm {} \;
find githooks -type f -exec ln -sf ../../{} .git/hooks/ \;

.PHONY: build
Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated for consistency

gssbzn
gssbzn previously approved these changes Jul 22, 2020
Copy link
Collaborator

@gssbzn gssbzn left a comment

Choose a reason for hiding this comment

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

one question and one small nit but none blocking

README.md Outdated Show resolved Hide resolved
scripts/check-version.sh Outdated Show resolved Hide resolved
Copy link
Collaborator

@gssbzn gssbzn left a comment

Choose a reason for hiding this comment

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

LGTM thanks for all the fixes

Copy link
Contributor

@PacoDw PacoDw left a comment

Choose a reason for hiding this comment

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

Nice! LGTM

Copy link
Collaborator

@themantissa themantissa left a comment

Choose a reason for hiding this comment

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

LGTM!

@MihaiBojin MihaiBojin merged commit 448f47a into mongodb:master Jul 23, 2020
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.

None yet

4 participants