Skip to content

Conversation

@wking
Copy link
Contributor

@wking wking commented May 19, 2016

I think this sort of generic hygiene advice lowers the signal/noise
for this file, since I'm more interested in what this project wants
that other projects may not want, and everybody wants well-described
PRs. And I suspect that folks who are inclined to post
poorly-described PRs are less likely to be reading CONTRIBUTING in
detail anyway ;).

Addresses my ocitools comment.

I think this sort of generic hygiene advice lowers the signal/noise
for this file, since I'm more interested in what this project wants
that other projects may not want, and everybody wants well-described
PRs.  And I suspect that folks who are inclined to post
poorly-described PRs are less likely to be reading CONTRIBUTING in
detail anyway ;).

Signed-off-by: W. Trevor King <[email protected]>
@crosbymichael
Copy link
Member

-1

Come on, like, I honestly don't know how to respond.

@wking
Copy link
Contributor Author

wking commented May 20, 2016

On Thu, May 19, 2016 at 05:11:53PM -0700, Michael Crosby wrote:

Come on, like, I honestly don't know how to respond.

A good starting point is pointing out disagreements with the argument
I used to motivate the change ;). In this case, you may find any of
the following unconvincing:

  • Talking about generally accepted behavior distracts from talking
    about project-specific behavior.
  • Clearly motivating pull requests is generally accepted behaviour.
  • Folk who are unaware of or knowingly flout that convention are
    unlikely to read CONTRIBUTING with enough care to do otherwise.

Those all sound reasonable to me, and I'm curious to know which don't
sound reasonable to you. Or if they all sound reasonable to you, but
you want to keep the line I'm removing for some other reason?

@hqhq
Copy link

hqhq commented May 20, 2016

Compare to describe pull request, I would prefer people write good commit logs, and I do think many people didn't realize that. They might write good pull request descriptions, but leave all commit logs empty. Maybe we can add something like that (if others agree this is useful).
EDIT: OK, I see we have guideline about commit messages already.

@wking
Copy link
Contributor Author

wking commented May 20, 2016

On Thu, May 19, 2016 at 11:31:41PM -0700, Qiang Huang wrote:

They might write good pull request descriptions, but leave all
commit logs empty. Maybe we can add something like that (if others
agree this is useful).

I don't mind if we document that or not. In projects I maintain, I
work around such submissions by adding my own docs in the commit that
merges the PR.

@cpuguy83
Copy link

I think the advice is helpful even if "obvious".
There are many things that are obvious, it doesn't mean they shouldn't be stated.
Things being "obvious" are based on experience, not some intrinsic knowledge.

@mlaventure
Copy link

-1 for me too.

It may be obvious for seasoned devs, but someone just starting contributing to Open Source Projects would probably read this document, and I'd rather not let them get bad habits :)

@wking
Copy link
Contributor Author

wking commented May 20, 2016

On Fri, May 20, 2016 at 10:12:01AM -0700, Brian Goff wrote:

There are many things that are obvious, it doesn't mean they
shouldn't be stated.

This is not black and white, and depends on how import something is,
how obvious it is, and how easy it is to recover from. For example,
“Write your commit message in a language that the maintainers speak”
is too obvious to be a useful CONTRIBUTING addition. “Motivate your
change in the commit message and PR summary” is less obvious, but easy
to recover from (just reply with a “Why do we want this?” comment).
Did this text initially land in the face of a flood of unmotivated PRs
from inexperienced devs? Did landing the text reduce the flood?

@vbatts
Copy link
Member

vbatts commented May 23, 2016

-1

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.

6 participants