search

OrlandoGolang / eps-conduit

Light-weight load balancer, written in Go
MIT License
2 stars 5 forks source link

Move contributing notes from README to CONTRIBUTING file #31

Closed dansackett closed 8 years ago

dansackett commented 8 years ago

Since we're using Github to host this, it would be nice to move the contribution notes in the README into a dedicated CONTRIBUTING guide like Github outlines here: https://help.github.com/articles/setting-guidelines-for-repository-contributors/. This would allow new Pull Requests to see the notes before making a PR and would keep the README for project notes.

scottcrespo commented 8 years ago

@dansackett

I never knew about this practice. You learn something new every day!

Any objections to a CONTRIBUTING file?

In addition, I need to expand the contribution notes to reflect discussions at the last Go meetup. In particular, members recommended contribution guidelines should contain testing requirements. I.e. "your new feature must contain unit tests or it will be rejected."

dansackett commented 8 years ago

@scottcrespo

I added a contributing.md (.md so Github renders the Markdown instead of displaying plaintext) to the Ellie project but copied the contributing guidelines from this repo to keep things consistent. I'm all for requiring tests before approval as it doesn't make sense to rely on the project maintainer to write tests for new features contributors add. I'd say once we have formal tests written here then we should add that part to the contribution guide. So a new issue to arise from this might be to implement basic tests.

Is there anything else discussed about contributions at the last meetup that I missed?

scottcrespo commented 8 years ago

@dansackett

From the meetup, we discussed code review standards. Here's what we put together:

Code Review

-- does it follow guidelines?

-- does it build?

-- does it run?

-- does it pass tests?

-- does it contain comments and/or documentation?

-- does it have a modular/sensible implementation?

dansackett commented 8 years ago

All of those seem good to me. In the "does it follow guidelines" category, is that for the PR based on the contribution notes or does it also include go fmt and other Go idioms being used? While most people use go fmt, I'll gladly deny a PR if a file hasn't been formatted correctly. Just checking if it should be another step in the process or if it's safely assumed in the above points.

scottcrespo commented 8 years ago

@dansackett

"does it follow guidelines" refers to the contribution guidelines. My intention was to include these code review standards as part of the guidelines, so people can see what we're looking for.

We could include a requirement about using go fmt. I'm all for it.

I'll make another draft that includes this material, and see if people like it.