Peer Reviews, Science & Relationships

I read an interesting comment on a forum the other day about feedback on a 'PR' (Pull Request).

For those that are unfamiliar, in some version control systems (eg :GIT), when multiple people are working on a project, individuals submit request to merge their latest updates with that of the 'main' branch. This is called 'Pull Request'.

Other developers and those leading the project can review the request, add comments and either approve or reject the update into the software update.

The idea, in principle, is simple -> Does this pull request improve the application in some way? Perhaps it makes something faster, or easier to understand (more intuitive) , perhaps it adds a missing feature, or fixes an annoying bug.

Those reviewing the request, can add their feedback to your work with the intent of building something 'more then the sum of it's parts' (aka synergy).

But what happens when the feedback received is simply a matter of personal opinion (subjective preference) as apposed to demonstrably verifiable improvement.

Should a personal preference/opinions be allowed? and if so should then prevent a 'PR' from being approved?

Building Relationships

If you imagine pull requests as a way of building relationships with other developers, it's easy to see how implementing their suggestions (opinions) could earn you some brownie points, and then with those brownies points, you gain favor, and perhaps your pull request is more likely to get approved.

In fact, even subjective opinions can be helpful, if for no other reason then they give you a new frame of reference to think about.  Consider the following

Both Sample A and Sample B produce the exact same output, and in the case many compilers will actually reduce both to the same machine code. It is essentially a 'personal preference', BUT what if the majority of your team mates opt for one over the other?

On one side, it is important at a minimum to remain consistent with the project, because when later maintaining the code base, it will be much easier if things are handled consistently. But on the other side, it really doesn't make much of a practical difference, but if you don't make the change, it might not get approved, and someone else will slip THE SAME CHANGE through only modified to match the style effectively missing out on your commit.

Science

Sometimes in our eagerness  I worry that we forget that second word in "Computer Science". Science, by its very nature is concerned with observation, hypothesis, and cause and effect relationships that effect the natural world.. What would happen if Einstien Theory of Relativity were impacted not by observation but leaned towards only 'the popular opinion'? Or what happens a scientific study on the the efficacy of a certain medication was tainted not by the observations but by corporate leaders out to make a quick buck? Maybe these things already happen? - and if so can we afford to continue to let this trend continue?

A Peer Review in Science is an impartial investigation of a body of work preformed by a scientist (or scientific team). It's purpose is continual improvement to ensure that the process of preforming the science upholds to well adopted methods of planning and execution. If done properly it should both validate and offer insight into the work. A critical analysis may find fault with a method used, measurements made, or even determine that the work does not hold up in a repeatable way.

Therefore, a pull request, (or any other type of software change proposal) should undergo an appropriate peer review process. One which aims to improve the quality of the code produced in some tangible, meaningful way.

Consider the following example

In this example, both algorithms sort an array of 'n' elements (or 0,n) in the case of Sample B

A peer reviewer can mathematically show the efficiency of SAMPLE B over SAMPLE A. Specifically for large sets of data, Sample B will most frequently and significantly outperform SAMPLE A. The effect is so large, that some fellow computer scientists suggest not even teaching SAMPLE A to students at all! In a praticle sense this could mean the difference between a user waiting 1 second vs 10 hours to sort the same set of data!!

This type of suggestion is an excellent example of continuous improvement, it both allows the program to improve in quality, and can even help teach new ways of thinking to the developer that might otherwise have gone undiscovered.

So while both 'Relationship building' and 'Science' have a role to play in the peer review process. I am of the opinion that it is important whenever possible to stick to the science when suggesting improvements to code.  Network over your lunch break, even have some fun coding Klingon style, but do science during your working hours.