From 5dc9f033d1f954eba690077410b38d021bb625eb Mon Sep 17 00:00:00 2001 From: Daniel Standage Date: Wed, 14 Mar 2018 15:01:54 -0700 Subject: [PATCH] Add some PR examples and pitch some cruft. Closes #464. --- doc/dev/getting-started.rst | 27 +++++++++++- doc/dev/guidelines-continued-dev.rst | 65 ---------------------------- 2 files changed, 26 insertions(+), 66 deletions(-) diff --git a/doc/dev/getting-started.rst b/doc/dev/getting-started.rst index fe29f39558..891920cf5c 100644 --- a/doc/dev/getting-started.rst +++ b/doc/dev/getting-started.rst @@ -141,6 +141,10 @@ Congratulations! You're ready to develop! Claiming an issue and starting to develop ----------------------------------------- +While consulting these instructions, you may find it helpful to refer to the +"Example pull requests" section below to see the workflow described here in +action. + #. Find an open issue and claim it. Go to `the list of open khmer issues `__ and find one you like; we suggest starting with `the low-hanging fruit issues `__). @@ -245,7 +249,8 @@ Claiming an issue and starting to develop git push origin -#. When you are ready to have the pull request reviewed, please mention @luizirber, @camillescott, @standage, @betatim, and/or @ctb with the comment 'Ready for review!' +#. When you are ready to have the pull request reviewed, post a comment with the text 'Ready for review!' + If desired, you can suggest specific developers or maintainers for review using an @ followed directly by their GitHub username. #. The khmer team will now review your pull request and communicate with you through the pull request page. Please feel free to add 'ping!' and an @ in the comments if you are looking for feedback—this will alert us that you are still on the line. @@ -260,6 +265,26 @@ Congratulations on making your first contribution to the khmer library! You're now an experienced GitHub user and an official khmer contributor! +Example pull requests +--------------------- + +If you are new to git or GitHub, you may find it helpful to see our pull request workflow in action. +Here are a few examples. + +- `Thread #1732 `_ is a very minimal pull request. + The first post includes a brief description of the small changes that were made, and indicated the changes were ready for peer review. + Following the initial post, the thread includes no discussion and is comprised almost entirely of automated messages generated by GitHub. + +- `Thread #1791 `_ likewise has very small changes to the code, but the thread includes a bit more discussion than the previous example. + The initial post includes some performance measurements, and subsequent comments include discussions of these numbers as well as requests for clarification on the code. + The author didn't explicitly request peer review of the code until the next day, but several people had already done an informal review of the code at that point. + +- `Thread #1792 `_ is a much more extensive pull request. + This PR addressed a core functionality in khmer, and accordingly the code changes were far reaching. + Over a month passed between when the PR was created and when the author requested final code review. + Along the way, however, the author requested feedback from other contributors, posted preliminary results, and posted many commits to the thread. + + After your first issue is successfully merged... ------------------------------------------------ diff --git a/doc/dev/guidelines-continued-dev.rst b/doc/dev/guidelines-continued-dev.rst index a367c1d5c2..8099b0dfcd 100644 --- a/doc/dev/guidelines-continued-dev.rst +++ b/doc/dev/guidelines-continued-dev.rst @@ -200,38 +200,6 @@ After this you'll have to add and commit the merge just like any other set of changes. It's also recommended that you run tests. -Virtual environments --------------------- - -The khmer package, like many software packages, relies on other third-party -software. Some of this software has been bundled together with khmer and is -compiled when you invoke ``make`` on the command line. But some of the software -khmer depends on is distributed as Python packages separately from khmer. - -Python `virtual environments `_ were -designed to isolate a stable development environment for a particular project. -This makes it possible to maintain different versions of a Python package for -different projects on your computer. - -The installation instructions in the :doc:`Getting Started ` -docs install the ``virtualenv`` command on your computer. After completing those -instructions, you can create a virtual environment with the command:: - - virtualenv -p python2 env/ - -(You can substitute `python3` for `python2` if Python version 3 is installed on -your system.) This command will create a new directory `env/` containing your -new virtual environment. The command:: - - source env/bin/activate - -will activate the virtual environment. Now any Python packages that you install -with ``pip`` or ``make install-dep`` will be installed into your isolated -virtual environment. - -Note that any time you create a new terminal session, using the virtual -environment requires that you re-activate it. - Pull request cleanup (commit squashing) --------------------------------------- @@ -261,36 +229,3 @@ See also `Code reviews: the lab meeting for code `__ and `the PyCogent coding guidelines `__. - -CPython Checklist ------------------ - -Here's a checklist for new CPython types with future-proofing for Python 3:: - - - [ ] the CPython object name is of the form `khmer_${OBJECTNAME}_Object` - - [ ] Named struct with `PyObject_HEAD` macro - - [ ] `static PyTypeObject khmer_${OBJECTNAME}_Type` with the following - entries - - [ ] `PyVarObject_HEAD_INIT(NULL, 0)` as the object init (this includes - the `ob_size` field). - - [ ] all fields should have their name in a comment for readability - - [ ] The `tp_name` filed is a dotted name with both the module name and - the name of the type within the module. Example: `khmer.ReadAligner` - - [ ] Deallocator defined and cast to `(destructor)` in tp_dealloc - - [ ] The object's deallocator must be - `Py_TYPE(obj)->tp_free((PyObject*)obj);` - - [ ] Do _not_ define a `tp_getattr` - - [ ] BONUS: write methods to present the state of the object via - `tp_str` & `tp_repr` - - [ ] _Do_ pass in the array of methods in `tp_methods` - - [ ] _Do_ define a new method in `tp_new` - - [ ] PyMethodDef arrays contain doc strings - - [ ] Methods are cast to `PyCFunctions`s - - [ ] Type methods use their type Object in the method signature. - - [ ] Type creation method decrements the reference to self - (`Py_DECREF(self);`) before each error-path exit (`return NULL;`) - - [ ] No factory methods. Example: `khmer_new_readaligner` - - [ ] Type object is passed to `PyType_Ready` and its return code is checked - in `MOD_INIT()` - - [ ] The reference count for the type object is incremented before adding - it to the module: `Py_INCREF(&khmer_${OBJECTNAME}_Type);`.