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

List archive Help


Re: [PeerLibrary dev] Do code reviews


Chronological Thread 
  • From: Mitar < >
  • To: PeerLibrary development < >
  • Subject: Re: [PeerLibrary dev] Do code reviews
  • Date: Fri, 01 Aug 2014 10:29:02 +0200

Hi!

I added a tag "code review needed" for pull requests in need of a code
review. This you should add to your pull request once it is ready for
code review (together with a comment so that you notify everyone):

https://github.com/peerlibrary/peerlibrary/labels/code%20review%20needed


Mitar

> 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


  • Re: [PeerLibrary dev] Do code reviews, Mitar, 08/01/2014

Archive powered by MHonArc 2.6.18.

Top of page