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

Delegate release notes to external tool #3055

Merged

Conversation

corneliusweig
Copy link
Contributor

@corneliusweig corneliusweig commented Oct 14, 2019

Description

This PR delegates the "heavy" lifting for creating the release changelog to an external tool hosted in my github space. I intend to maintain this tool and not break the formatting. @balopat also has write access to this repository and was involved in factoring out this piece of code.

The hack/release_notes.sh script installs the external helper program as hack/release-notes, if not present.

User facing changes

n/a

Next PRs.

n/a

Reviewer Notes

  • The code flow looks good.
  • Unit test added.
  • User facing changes look good.

@codecov
Copy link

codecov bot commented Oct 14, 2019

Codecov Report

Merging #3055 into master will not change coverage.
The diff coverage is n/a.

hack/release.sh Outdated
release_notes_workdir="$(mktemp -d)"
trap 'rm -rf -- ${release_notes_workdir}' RETURN

# See https://stackoverflow.com/questions/56842385/using-go-get-to-download-binaries-without-adding-them-to-go-mod for this workaround
Copy link
Contributor

Choose a reason for hiding this comment

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

I would just add the tool to go mods!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can you explain a little how to do that? If I just add it with go get .../release-notes, then it will be lost on the next go mod tidy. Besides, I don't think this would install the tool in $GOPATH/bin or so.

Copy link
Contributor

Choose a reason for hiding this comment

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

@corneliusweig One way to do it is to create a top level tools folder that is a sub-package, with it's own go.mod. In that folder, you add a tools.go file that imports xxx/release-notes. You can then run go mod vendor in that folder.
In the makefile, you'd run cd tools; go install -mod=vendor xxx/release-notes

Copy link
Contributor

@dgageot dgageot Oct 16, 2019

Choose a reason for hiding this comment

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

Here's a sample with golangci-lint: https://github.com/buildpack/pack/pull/299/files

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dgageot oh.. that's a nice way. I tried to stay away from vendor mode though, it seems to work ok either way.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why did you avoid vendoring? The advantage is that there's nothing to download on the Travis. It's a bit quicker and more resilient.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was adding 11MB of vendored code. Besides, it is only run on developer machines, right? So will not affect Travis anyway.

I know that you are not a big fan of go modules, so if you have a strong opinion in this case, I won't argue any longer :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Not a big fan indeed. In that case, it's actually very convenient. You are right that it doesn't run on Travis so, yes, vendoring is not so useful. Good point!

@tejal29 tejal29 added the kokoro:run runs the kokoro jobs on a PR label Oct 21, 2019
@kokoro-team kokoro-team removed the kokoro:run runs the kokoro jobs on a PR label Oct 21, 2019
@balopat balopat merged commit 5a78a0e into GoogleContainerTools:master Oct 28, 2019
@corneliusweig corneliusweig deleted the w/release-notes-helper branch October 29, 2019 19:10
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.

6 participants