DOC: Updating contributing docs #1052

Merged
merged 3 commits into from Oct 23, 2013

Projects

None yet

3 participants

@jseabold
Member

Also adds a stub for the 0.6. release notes so we can ask people to fill in as we go this time.

Would be good to get another set of eyes on the minimum requirements for accepting a PR. Anything else?

Hopefully, this ups the ante a bit for getting PRs merged -- mainly focusing more on documentation and editing the release notes. This will improve the quality of the code base and make it easier to just tag and release when ready.

@guyrt guyrt commented on the diff Aug 28, 2013
CONTRIBUTING.rst
1. `Fork <https://help.github.com/articles/fork-a-repo>`_ the `statsmodels repository <https://github.com/statsmodels/statsmodels>`_ on Github.
-2. `Create a new feature branch <http://git-scm.com/book/en/Git-Branching-Basic-Branching-and-Merging>`_
- + Each branch must be self-contained, with a single new feature or bugfix.
- + Patches must always include tests. See our `notes on testing <test_notes.html>`_.
@guyrt
guyrt Aug 28, 2013 Contributor

This link 404ed for me. Looks like it shouldn't be using https.

edit: I meant test_notes.html

@jseabold
jseabold Aug 28, 2013 Member

Right. Thanks. This one is malformed too, because it won't expand the sphinx shortcut here.

@josef-pkt josef-pkt commented on an outdated diff Oct 22, 2013
CONTRIBUTING.rst
-Statsmodels is released under the
-`Modified (3-clause) BSD license <http://www.opensource.org/licenses/BSD-3-Clause>`_.
+- Include a short, self-contained code snippet that reproduces the problem
+- Specify the statsmodels version used. You can do this with ``sm.version.full_version``
+- If the issue looks to involve other dependencies, also include the output of ``sm.show_versions()``
+
+Making Changes to the Code
+~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+For a pull request to be accepted, you must meet the below requirements. This greatly helps in keeping the job of maintaining and releasing the software a shared effort.
+
+- **One branch. One feature.** Branches are cheap and github makes it easy to merge and delete branches with a few clicks. Avoid the temptation to lump in a bunch of unrelated changes when working on a feature, if possible. This helps us keep track of what has changed when preparing a release.
+- Commit messages should be clear and concise. This means a subject line of less than 80 characters, and, if necessary, a blank line followed by a commit message body. We have an `informal commit format standard <http://statsmodels.sourceforge.net/devel/dev/maintainer_notes.html#commit-comments>`_ that we try to adhere to. You can see what this looks like in practice by ``git log --oneline -n 10``. If your commit references or closes a specific issue, you can close it by mentioning it in the `commit message <https://help.github.com/articles/closing-issues-via-commit-messages>`_. (*For maintainers*: These suggestions go for Merge commit comments too. These are partially the record for release notes.)
+- Code submissions must always include tests. See our `notes on testing <http://statsmodels.sourceforge.net/devel/dev/test_notes.html>`_.
@josef-pkt
josef-pkt Oct 22, 2013 Member

maybe tone down the "must" a bit
for example "Code submissions need to have unit tests before they can be merged"

@josef-pkt
Member

looks good to merge, except that I don't like the word "must" when it's based on volunteer work

@josef-pkt
Member

I added milestole 0.6 unless you want to backport

@jseabold
Member

Rebased.

@jseabold jseabold merged commit f6ea129 into statsmodels:master Oct 23, 2013

1 check was pending

default The Travis CI build is in progress
Details
@jseabold jseabold deleted the jseabold:contrib-docs branch Oct 23, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment