spec: protobuf declares syntax and package name

[jdef]

No description provided.

[jieyu]

LGTM. Thanks!

[jdef]

@akutz PTAL

[akutz]

LGTM (https://gist.github.com/jdef/57c2759a9e57bc9353f4cd1aafcfd134)

[akutz]

@jdef / @jieyu / @saad-ali,

I just noticed that this PR comes from a branch on this repo. I strongly suggest you use a fork as others will be doing so. You should dog-food your process and not create PRs directly on the project's repo unless they are release or support branches.

[jdef]

Good feedback @akutz. I'll submit future PRs from a forked repo.

[akutz]

Hi @jdef,

Additionally, your commit message's body should not exceed 72 characters in order to be a valid Git message body. I like this blog for info on Git commit messages.

[akutz]

Also, according to @jieyu all text should be properly cased and full, English sentences. The commit message's subject and body do not adhere to that. I concur that intent is what matters, but @jieyu seemed very serious about every place I did not adhere to this rule, so I thought I'd mention it here as well.

[jdef]

Your attention to detail is appreciated. Overlooked the git-commit rules in my haste to unblock other PRs. Would be nice if GH provided a better commit message editor.

…

On Fri, Aug 11, 2017 at 3:39 PM, Schley Andrew Kutz < ***@***.***> wrote: Also, according to @jieyu <https://github.com/jieyu> all text should be properly cased and full, English sentences. The commit message's subject and body do not adhere to that. I concur that intent is what matters, but @jieyu <https://github.com/jieyu> seemed very serious about every place I did not adhere to this rule, so I thought I'd mention it here as well. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#78 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACPVLP_XXeuvm9FM18Tl0oCuv6EyDv1fks5sXK30gaJpZM4O1Bu6> .

[akutz]

Hi @jdef,

Perhaps more time reviewing the PR prior to a merge? I was surprised @jieyu missed it :) As far as Git commit message editors, I use Vim and it has built-in support for indicating when the Git message and body width limits are exceeded. Just set this in your profile and Git will use Vim to edit commit messages:

export EDITOR=vim

[akutz]

I also recommend, as I did to @jieyu the other day with his style-related PR, that PRs should have meaningful descriptions. Perhaps simply copy the commit message's body? A PR with an empty description is a bad look IMO.

[jieyu]

Sorry, I overlooked the commit body. Probably we should add a commit hook to check that.

[akutz]

Or just edit them in Vim and rely on it handling it for you. It's not difficult to check locally.

[jieyu]

I guess @jdef hit squash and merge button from github, thus having this issue.

[akutz]

Ugh. I don't trust that myself. How do you guys like it?

[jdef]

Not as precise as vim. Does the job in a pinch but, as you've observed, makes it easy to break rules..

…

On Fri, Aug 11, 2017 at 4:32 PM, Schley Andrew Kutz < ***@***.***> wrote: Ugh. I don't trust that myself. How do you guys like it? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#78 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACPVLFdGZYvW0F4PU5jPQ0h1ACF0pNTmks5sXLpKgaJpZM4O1Bu6> .