38 \ December 2018 \ http://www.phparch.comProducing Packages, Part Three
The Workshop
known issues with providers or environments, and any other
issue I may be able to spot easily.
We can add an issue template for our library by creating the
following file in our repository: .github/ISSUE_TEMPLATE.md.
The file ISSUE_TEMPLATE.md is a simple text file in Markdown
format which the user sees when they open a new issue. They
don’t have to copy and paste anything; they only have to fill
out the template and submit.
Sometimes users will disregard your template and write out
their issues. Being patient with users like this may be difficult.
I often close their issue and politely request they follow the
issue template to describe their problem fully. I’ve never had
a case where a legitimate user refused to fill out a template.
Maybe I’m just lucky? Always assume the user on the other
end has good intentions. You can’t hear the emotion and
inflection in their voice from reading their written words. Try
not to read into what the user is saying. Instead, assume the
best, but prepare for the worst.
Dealing With Pull Requests
Since we’re working on open source code, at some point
someone may want to send us a pull request. A pull request
is a formal change request to merge changes from another
branch or another branch on someone else’s copy of your repo
(also known as a fork). It is our responsibility as open source
maintainers to review and give feedback on pull requests. If
the pull request adds some great new feature, it’s free code for
us! If the pull request is changing too much code, we should
be tactful and supportive in our feedback. Always provide
constructive criticism. Don’t tell anyone what they did wrong,
tell them what they could do better.
In Part One of this series we wired up Travis-CI^3 to run
our tests every time we pushed a commit and when someone
opens a pull request. Travis becomes our pull request gate-
keeper. If the changes we are reviewing in the pull request
cause the tests to fail, we know there is a problem. The prob-
lem could be the incoming pull request fixes a bug and we
need to add or update our tests. The problem could also be
the user didn’t update the tests when they made the change.
Another possible scenario is the user’s change broke some-
thing the existing tests caught. When this happens, you
should give the user some feedback on how to fix their code
or how to update the tests (if it’s not evident).
Always encourage contributors to fix, add, and update tests
in addition to the other code changes they have made. We
don’t need to be strict about this. After all, they are doing us
a favor by contributing their time and code to our project.
If the tests all pass and the contributor added new tests to
cover any functionality they have added we’re in good shape!
If the contributor has not added tests and they have added
functionality we should gently ask them to add supporting
tests. If they reply with, “I don’t know how” please go easy on
them! Everyone learns at their own rate. If you have the time,
help them through adding tests. If they are not receptive or
3 Travis-CI: https://travis-ci.org
stop replying, consider accepting the change as is and adding
the tests yourself.
No matter the outcome of the pull request always take the
time it takes to type out “Thanks for opening this pull request”
or “thanks for your time working on this.” Always remember
someone else took the time to work on your library. They are
doing you a favor (hopefully).Acceptance Checklist
The things I like to check for in any pull request are listed
here.Do I want this change?
Is this change something which gives the library more
functionality? Does it fit with what the rest of the package
does? Does it make sense? If it adds dependencies, do they
make sense, are they up to date, and actively maintained?Do the Tests Pass?
Did the author add or remove tests based on their changes?
Spot check to make sure the author wrote all the necessary
tests to ensure the new feature is tested with successful condi-
tions as well as proving failure is handled as expected. At
the same time, look at the code style of the new code, does
it follow the established style guide? If not, we can always
default to checking if the code is PSR-2^4 compliant.
What Feedback Do I Have for the Author?
GitHub has a fantastic review system for pull requests. You
can also request a review from someone in the organization
if you’d like. I’m trying to get in the habit of using this feature
because the outcomes are “Approve”, “Request Changes”, and
“Comment” which give the author a good sense of what needs
to be done (if anything).
Here you can easily request the author change functionality,
add test cases, or further discuss what you’d rather see. Feel
free to add simple comments or discuss changes inline—the
review system is quite helpful for both sides of the pull request.
If you are using Bitbucket or GitLab, make sure you
comment inline or as regular comments on the pull request.Handling a PR You Want
Pull requests you want are easy to handle. By this point,
we’ve already ensured the tests are passing, and the author
has added tests as needed. Depending on how large the pull
request is you may want to pull down the changes to look at
them on your system in your editor. To do so, we run two
commands to checkout the code:git checkout -b reviewing-feature master
git pull [email protected]:svpernova09/php-easy-math.git \
reviewing-featureNow we have a new branch reviewing-feature in our local
folder with all of the changes. From here we can test to our
heart’s content.4 PSR-2: https://www.php-fig.org/psr/psr-2