1(core_dev)= 2 3Core Developer Guide 4==================== 5 6Welcome, new core developer! The core team appreciate the quality of 7your work, and enjoy working with you; we have therefore invited you 8to join us. Thank you for your numerous contributions to the project 9so far. 10 11This document offers guidelines for your new role. First and 12foremost, you should familiarize yourself with the project's 13{doc}`mission, vision, and values <values>`. When in 14doubt, always refer back here. 15 16As a core team member, you gain the responsibility of shepherding 17other contributors through the review process; here are some 18guidelines. 19 20All Contributors Are Treated The Same 21------------------------------------- 22 23You now have the ability to push changes directly to the main 24branch, but should never do so; instead, continue making pull requests 25as before and in accordance with the 26{doc}`general contributor guide <contribute>`. 27 28As a core contributor, you gain the ability to merge or approve 29other contributors' pull requests. Much like nuclear launch keys, it 30is a shared power: you must merge *only after* another core has 31approved the pull request, *and* after you yourself have carefully 32reviewed it. (See {ref}`sec:reviewing` and especially 33{ref}`sec:understand` below.) To ensure a clean git history, 34use GitHub's [Squash and Merge][gh_sqmrg] 35feature to merge, unless you have a good reason not to do so. 36 37[gh_sqmrg]: https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/merging-a-pull-request#merging-a-pull-request-on-github 38 39(sec:reviewing)= 40Reviewing 41--------- 42 43### How to Conduct A Good Review 44 45*Always* be kind to contributors. Nearly all of `scikit-image` is 46volunteer work, for which we are tremendously grateful. Provide 47constructive criticism on ideas and implementations, and remind 48yourself of how it felt when your own work was being evaluated as a 49novice. 50 51`scikit-image` strongly values mentorship in code review. New users 52often need more handholding, having little to no git 53experience. Repeat yourself liberally, and, if you don’t recognize a 54contributor, point them to our development guide, or other GitHub 55workflow tutorials around the web. Do not assume that they know how 56GitHub works (e.g., many don't realize that adding a commit 57automatically updates a pull request). Gentle, polite, kind 58encouragement can make the difference between a new core developer and 59an abandoned pull request. 60 61When reviewing, focus on the following: 62 631. **API:** The API is what users see when they first use 64 `scikit-image`. APIs are difficult to change once released, so 65 should be simple, [functional][wiki_functional] (i.e. not 66 carry state), consistent with other parts of the library, and 67 should avoid modifying input variables. Please familiarize 68 yourself with the project's [deprecation policy][dep_pol] 69 702. **Documentation:** Any new feature should have a gallery 71 example, that not only illustrates but explains it. 72 733. **The algorithm:** You should understand the code being modified or 74 added before approving it. (See {ref}`sec:understand` 75 below.) Implementations should do what they claim, 76 and be simple, readable, and efficient. 77 784. **Tests:** All contributions to the library *must* be tested, and 79 each added line of code should be covered by at least one test. Good 80 tests not only execute the code, but explores corner cases. It is tempting 81 not to review tests, but please do so. 82 83[wiki_functional]: https://en.wikipedia.org/wiki/Functional_programming 84[dep_pol]: https://scikit-image.org/docs/dev/contribute.html#deprecation-cycle 85 86Other changes may be *nitpicky*: spelling mistakes, formatting, 87etc. Do not ask contributors to make these changes, and instead 88make the changes by [pushing to their branch][gh_push] 89or using GitHub’s [suggestion][gh_suggest] [feature][gh_feedback]. 90(The latter is preferred because it gives the contributor a choice in 91whether to accept the changes.) 92 93[gh_push]: https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/committing-changes-to-a-pull-request-branch-created-from-a-fork 94[gh_suggest]: https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/commenting-on-a-pull-request 95[gh_feedback]: https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/incorporating-feedback-in-your-pull-request 96 97Our default merge policy is to squash all PR commits into a single 98commit. Users who wish to bring the latest changes from ``main`` 99into their branch should be advised to merge, not to rebase. Even 100when merge conflicts arise, don’t ask for a rebase unless you know 101that a contributor is experienced with git. Instead, rebase the branch 102yourself, force-push to their branch, and advise the contributor on 103how to force-pull. If the contributor is no longer active, you may 104take over their branch by submitting a new pull request and closing 105the original. In doing so, ensure you communicate that you are not 106throwing the contributor's work away! 107 108Please add a note to a pull request after you push new changes; GitHub 109does not send out notifications for these. 110 111(sec:understand)= 112### Merge Only Changes You Understand 113 114*Long-term maintainability* is an important concern. Code doesn't 115merely have to *work*, but should be *understood* by multiple core 116developers. Changes will have to be made in the future, and the 117original contributor may have moved on. 118 119Therefore, *do not merge a code change unless you understand it*. Ask 120for help freely: we have a long history of consulting community 121members, or even external developers, for added insight where needed, 122and see this as a great learning opportunity. 123 124While we collectively "own" any patches (and bugs!) that become part 125of the code base, you are vouching for changes you merge. Please take 126that responsibility seriously. 127 128In practice, if you are the second core developer reviewing and approving a 129given pull request, you typically merge it (again, using GitHub's Squash and 130Merge feature) in the wake of your approval. What are the exceptions to this 131process? If the pull request has been particularly controversial or the 132subject of much debate (e.g., involving API changes), then you would want to 133wait a few days before merging. This waiting time gives others a chance to 134speak up in case they are not fine with the current state of the pull request. 135Another exceptional situation is one where the first approving review happened 136a long time ago and many changes have taken place in the meantime. 137 138When squashing commits GitHub concatenates all commit messages. 139Please edit the resulting message so that it gives a concise, tidy 140overview of changes. For example, you may want to grab the 141description from the PR itself, and delete lines such as "pep8 fix", 142"apply review comments", etc. Please retain all Co-authored-by 143entries. 144 145Closing issues and pull requests 146-------------------------------- 147 148Sometimes, an issue must be closed that was not fully resolved. This can be 149for a number of reasons: 150 151- the person behind the original post has not responded to calls for 152 clarification, and none of the core developers have been able to reproduce 153 their issue; 154- fixing the issue is difficult, and it is deemed too niche a use case to 155 devote sustained effort or prioritize over other issues; or 156- the use case or feature request is something that core developers feel 157 does not belong in scikit-image, 158 159among others. Similarly, pull requests sometimes need to be closed without 160merging, because: 161 162- the pull request implements a niche feature that we consider not worth the 163 added maintenance burden; 164- the pull request implements a useful feature, but requires significant 165 effort to bring up to scikit-image's standards, and the original 166 contributor has moved on, and no other developer can be found to make the 167 necessary changes; or 168- the pull request makes changes that do not align with our values, such as 169 increasing the code complexity of a function significantly to implement a 170 marginal speedup, 171 172among others. 173 174All these may be valid reasons for closing, but we must be wary not to alienate 175contributors by closing an issue or pull request without an explanation. When 176closing, your message should: 177 178- explain clearly how the decision was made to close. This is particularly 179 important when the decision was made in a community meeting, which does not 180 have as visible a record as the comments thread on the issue itself; 181- thank the contributor(s) for their work; and 182- provide a clear path for the contributor or anyone else to appeal the 183 decision. 184 185These points help ensure that all contributors feel welcome and empowered to 186keep contributing, regardless of the outcome of past contributions. 187 188Further resources 189----------------- 190 191As a core member, you should be familiar with community and developer 192resources such as: 193 194- Our {doc}`contributor guide <contribute>` 195- Our [community guidelines](https://scikit-image.org/community_guidelines.html) 196- [PEP8](https://www.python.org/dev/peps/pep-0008/) for Python style 197- [PEP257](https://www.python.org/dev/peps/pep-0257/) and the 198 [NumPy documentation guide][numpydoc] 199 for docstrings. (NumPy docstrings are a superset of PEP257. You 200 should read both.) 201- The scikit-image [tag on StackOverflow][so_tag] 202- The scikit-image [tag on forum.image.sc](https://forum.image.sc/tags/scikit-image) 203- Our [developer forum][ml] 204- Our [chat room](https://skimage.zulipchat.com/) 205 206[numpydoc]: https://docs.scipy.org/doc/numpy/docs/howto_document.html 207[so_tag]: https://stackoverflow.com/questions/tagged/scikit-image 208[ml]: https://discuss.scientific-python.org/c/contributor/skimage 209 210You are not required to monitor all of the social resources. 211 212Inviting New Core Members 213------------------------- 214 215Any core member may nominate other contributors to join the core team. 216Nominations happen on a private email list, 217<skimage-core@python.org>. As of this writing, there is no hard-and-fast 218rule about who can be nominated; at a minimum, they should have: been 219part of the project for at least six months, contributed 220significant changes of their own, contributed to the discussion and 221review of others' work, and collaborated in a way befitting our 222community values. 223 224Contribute To This Guide! 225------------------------- 226 227This guide reflects the experience of the current core developers. We 228may well have missed things that, by now, have become second 229nature—things that you, as a new team member, will spot more easily. 230Please ask the other core developers if you have any questions, and 231submit a pull request with insights gained. 232 233Conclusion 234---------- 235 236We are excited to have you on board! We look forward to your 237contributions to the code base and the community. Thank you in 238advance! 239