General mailing list for discussions and development of PeerLibrary and related software.

List archive Help


[PeerLibrary dev] Do code reviews


Chronological Thread 
  • From: Mitar < >
  • To: PeerLibrary development < >
  • Subject: [PeerLibrary dev] Do code reviews
  • Date: Mon, 21 Jul 2014 14:05:40 +0200

Hi!

Please do code reviews of pending pull requests. Currently it feels like
I am the only one doing them. :-( And this is not good because it makes
me the only one doing it and in some way a gatekeeper (and a bad one as
well, when I do not have time to do it). Reviewing pull requests is also
a great way to learn more about our codebase and learn from others how
they are tackling programming in PeerLibrary. You can also just do a
code review and ask questions, why is something like that and not
different, questions if you do not understand something (maybe a code
comment is then missing)? And also read comments from others and learn
ways to think about the code and project.

https://github.com/peerlibrary/peerlibrary/pulls

So our development model is described on wiki
(https://github.com/peerlibrary/peerlibrary/wiki/Development-Model), but
in short the idea is to use feature branch -> development branch pull
requests inside our repository (not from the repository fork) to do a
form of pair-programming. The idea is mostly to have at least two people
see each line of the code before it goes in.

One starts working on a new feature, creates a feature branch, and
creates a pull request. Then works. And then at some point requests
others to make a code review (you can already read and comment code
before, but if branch is not yet finished, this could be premature - of
course, if you see or think you see that somebody is going into a
"wrong" (in your opinion) direction, open a discussion inside the pull
request).

When you are code reviewing branch on which somebody else is working on,
it is best to write an inline GitHub comment (especially if some
discussion seems first needed). But sometimes it is easier or better
just to add a commit to the branch with a fix or change you have in
mind. In any case, anything can be changed, iterated upon, improved. So
do not worry of breaking anything. We can all just improve. No code is
"owned" by anybody so feel good suggesting modifications, or helping by
modifying it.

It is better to ask for forgiveness than permission.

Only one thing: never ever use "force" switch when pushing to main
repository. We prefer having many commits which were later reverted or
fixed, than losing commits. If it is really necessary, consult first
with other developers.

This pull requests would need some code review love, for example:

https://github.com/peerlibrary/peerlibrary/pull/520
https://github.com/peerlibrary/peerlibrary/pull/550

This one is one which is just being ready to be merged in, so you can
see how a reviewed one looks like:

https://github.com/peerlibrary/peerlibrary/pull/538


Mitar

--
http://mitar.tnode.com/
https://twitter.com/mitar_m


  • [PeerLibrary dev] Do code reviews, Mitar, 07/21/2014

Archive powered by MHonArc 2.6.18.

Top of page