Skip to content

Conversation

@tbg
Copy link
Member

@tbg tbg commented Dec 2, 2017

Targeted alternative (or perhaps short-term stand-in) for 1.
I think it's worth linting this sooner rather than later.

Release notes: none.

Targeted alternative (or perhaps short-term stand-in) for [1].
I think it's worth linting this sooner rather than later.

Release notes: none.

[1]: cockroachdb#18782
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@tbg tbg requested a review from benesch December 2, 2017 00:04
@jordanlewis
Copy link
Member

It would be great if this could come with a post-commit hook too.

@tbg
Copy link
Member Author

tbg commented Dec 4, 2017

Yeah, that would be ideal, but that's the kind of work already done in #18782 (except in the context of a general commit linter, which I'm not really on board with). You can't just run a lint as a post-commit hook because it's way too slow. Perhaps @benesch can reduce #18782 to the uncontroversial part (linting release notes) and land it?

Either way, I'm pretty confident that linting these today rather than next week makes sense.

Copy link
Member

@jordanlewis jordanlewis left a comment

Choose a reason for hiding this comment

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

I just meant a little git post-commit hook that checks for the existence of the
Release notes line and prints a warning if it's missing. I don't want to have
to build or run any Go test program to just grep my commit message.

I suppose we can do this offline since you can't add git hooks via a repository
commit anyway.

@benesch
Copy link
Contributor

benesch commented Dec 4, 2017 via email

@tbg
Copy link
Member Author

tbg commented Dec 4, 2017

Heard @jordanlewis is cooking something up that has better ergonomics, closing this one!

@tbg tbg closed this Dec 4, 2017
@tbg tbg deleted the lint/release-note branch December 4, 2017 16:22
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.

4 participants