Skip to content

Better contributing guidelines#826

Merged
cobyism merged 4 commits intogithub:masterfrom
cobyism:rework-contributing-guides
Nov 5, 2013
Merged

Better contributing guidelines#826
cobyism merged 4 commits intogithub:masterfrom
cobyism:rework-contributing-guides

Conversation

@cobyism
Copy link
Copy Markdown
Contributor

@cobyism cobyism commented Nov 4, 2013

I’m proposing that we articulate the contributing guidelines for this project better, with the aim of making the maintaining of this repository easier in the long term. By making some changes along the lines this PR proposes, I’m hoping that future pull requests will be easier to review, and that the large number of currently open pull requests can be more easily worked through once we have a more solid set of criteria for accepting changes to the project.

In essence, the main change here is just requesting that all pull requests are accompanied by:

  • Links to the projects, apps, tools, languages, or frameworks the change applies to.
  • Links to supporting documentation about the ignored files, or a good description of how they relate to everything else if no documentation is available.
  • Description (even if a short one) for why the change should be made.

I guess what I’m hoping this cuts down on is pull requests which have only vague titles and no description/comments, or don’t provide the necessary additional information to aid maintainers (who may lack direct experience with the applicable language/framework/tool/project) in making decisions about how to proceed. I believe that the volume of these kind of pull requests is what has contributed most to the recent lack of maintenance of this project—I’m not suggesting these things for the sake of being difficult, it’s out of ❤️ and a desire to see this project take a turn for the better.

View the rendered README at this branch.

Thoughts?

/cc @niik @jspahrsummers @aroben @haacked @izuzak Since I believe you’ve all expressed interest (in various ways) in helping maintain this repository.

Comment thread README.md Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I wonder if we could rephrase this section to be about just adding ignore rules for one framework/tool/language at a time. Not sure if it's possible or how to phrase it though. I see quite a few PRs which tries to add ignore rules for two frameworks or tools where I'd be happy to accept one of them but I'd like to defer on the other to someone with more knowledge about that particular tool.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I think that’s a good call. This particular guideline was in the old README, and the way I read it was more of a pragmatic guideline because of merge conflicts rather than as a guideline to make deciding which PRs to merge easier. That said, I’d far rather people kept PRs atomic than worry about merge conflicts, so I think your suggestion is spot on.

@niik
Copy link
Copy Markdown
Member

niik commented Nov 4, 2013

⚡ ✨ ❤️ @cobyism! Thanks for writing this up. I'm watching this repo now and I'll make an effort to handle incoming PRs related to windows, visual studio and .net in general. I added one tiny comment which is actually kinda douchy since I don't have a clue about how to make it happen so if it doesn't make sense to you just ignore it.

@niik
Copy link
Copy Markdown
Member

niik commented Nov 4, 2013

Forgot to mention this. I've seen a few PRs for ignoring files and directories produced by tools that hardly anyone uses. Case by case is probably enough but it'd make reviewing additions easier if we had some guideline around not accepting every random new-project *.ext ignore rule coming in. I'm probably overly cautious seeing as GitHub for Windows ships with a lot of these rules and I don't want to break peoples repos. Thoughts?

@niik
Copy link
Copy Markdown
Member

niik commented Nov 4, 2013

Also it'd be helpful to have some guidelines about new top-level files. If they automatically get pulled in to dotcom (do they) we probably want to be cautious about not bloating that list.

@cobyism
Copy link
Copy Markdown
Contributor Author

cobyism commented Nov 4, 2013

I've seen a few PRs for ignoring files and directories produced by tools that hardly anyone uses.
… if we had some guideline around not accepting every random new-project *.ext ignore rule coming in

Yeah, this is a concern of mine too. In general I want to err on the side of quality than quantity/coverage, but I think a case-by-case basis is the only real way to deal with it. I think raising the bar in terms of requiring people to link to homepages and documentation and so forth will help though, because it will be easier to filter out the long-tail of tools which are less likely to have up-to-date docs.

If they automatically get pulled in to dotcom (do they) we probably want to be cautious about not bloating that list.

They don’t get automatically pulled in, but I definitely agree that in general we should keep new templates to a minimum, adding templates only when there’s a clear case for their inclusion. 👍

@aroben
Copy link
Copy Markdown
Contributor

aroben commented Nov 5, 2013

Should we update CONTRIBUTING.md too?

Another thought I've had is that we should consider adding collaborators to this repo for editors/languages/frameworks we (GitHub) aren't personally familiar with.

@cobyism
Copy link
Copy Markdown
Contributor Author

cobyism commented Nov 5, 2013

Should we update CONTRIBUTING.md too?

@aroben Yep, I was planning on extracting out the guidelines from the README once we’re happy with the content.

we should consider adding collaborators to this repo for editors/languages/frameworks we (GitHub) aren't personally familiar with.

That’d be great, but I’m not really sure what the best way to encourage that will be. Also, there’s such a wide range of technology represented in the current set of templates, is there enough overlap between templates pertaining to certain languages or technologies to warrant separate maintainers per-technology? I feel like maybe if there are people who over time obvious submit multiple changes they’ll become obvious people to approach, but do we need to say anything explicit about looking to involve others to help maintain the project too?

@aroben
Copy link
Copy Markdown
Contributor

aroben commented Nov 5, 2013

there’s such a wide range of technology represented in the current set of templates, is there enough overlap between templates pertaining to certain languages or technologies to warrant separate maintainers per-technology?

Huh? That sounds like an argument for having per-technology maintainers. Lots of technologies ➡️ lots of maintainers.

do we need to say anything explicit about looking to involve others to help maintain the project too?

I don't think so. I think we should consider adding folks as collaborators after even just one or two merged pull requests. We don't have to go as far as http://felixge.de/2013/03/11/the-pull-request-hack.html, but I don't think we need to make the barrier to entry super high.

@cobyism
Copy link
Copy Markdown
Contributor Author

cobyism commented Nov 5, 2013

Lots of technologies -> lots of maintainers.

Sorry, I didn’t articulate myself very well before. What I was trying to convey was that this project has ~90 project-specific templates and ~30 global templates, the vast majority of which are not things we (GitHub) are familiar with. Is it really a good idea to try and find maintainers for each one? It might be, but it just sounds like that could get kinda gnarly pretty quickly. I have far less experience than most when it comes to maintaining open source projects though, so I’ll happily defer on things like this 😄

I think we should consider adding folks as collaborators after even just one or two merged pull requests.
I don't think we need to make the barrier to entry super high.

Cool, that sounds like a good plan. 👍

@haacked
Copy link
Copy Markdown
Contributor

haacked commented Nov 5, 2013

We don't have to go as far as http://felixge.de/2013/03/11/the-pull-request-hack.html, but I don't think we need to make the barrier to entry super high.

👍 We can even look at historical data to identify people we may want to give access to. If someone has shown good judgment, I think we should give them commit access to help out.

@cobyism
Copy link
Copy Markdown
Contributor Author

cobyism commented Nov 5, 2013

Aight, are people happy for me to merge this in so we can start pointing people to it? If so I’ll update the CONTRIBUTING.md file and hit the button.

@aroben
Copy link
Copy Markdown
Contributor

aroben commented Nov 5, 2013

:shipit:

cobyism pushed a commit that referenced this pull request Nov 5, 2013
@cobyism cobyism merged commit 6188909 into github:master Nov 5, 2013
@cobyism cobyism deleted the rework-contributing-guides branch November 5, 2013 17:06
@jspahrsummers
Copy link
Copy Markdown
Contributor

💎

@jspahrsummers
Copy link
Copy Markdown
Contributor

@aroben @cobyism In the past, @kneath made the point that adding more collaborators will probably just make this repo even crazier. Most of the ignore templates are fairly complete at this point, so there's likely not much benefit to accepting PRs at an increased rate.

drothmaler pushed a commit to drothmaler/gitignore that referenced this pull request May 27, 2014
u9E9F pushed a commit to u9E9F/gitignore that referenced this pull request Jul 8, 2017
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