Skip to main content
Topic: Code reviews notes (Read 6089 times) previous topic - next topic
0 Members and 1 Guest are viewing this topic.

Code reviews notes

Is it just me, or we're more in them refactoring times, all of us? :D
The past days PRs seem to deal with a number of refactorings everywhere by all. While most are localized (somewhat small), Ema's are wide :D
Regardless how localized, the overall result is: they improve significantly the codebase we later work with.

And there's a word which comes with them: code reviews! :)
The more, the better. You never know what bit, small or big, another pair of eyes on the code will notice. Even questions or discussion on the code, have an extra chance to make it better, to spin off in a cleaner solution.

Ways for reviews:
1) on the web interface.
Good at spotting some categories of potential issues, not so good at others.

2) check out the PR locally.
Ema's bookmarks come in handy here, with this bit:
https://gist.github.com/piscisaureus/3342247
(perhaps we can find even a better one or rewrite this one clearly?)
We only need to do it once on a repo, to register the heads for the PRs in the config file. Then, when you fetch --all, you'll be able to checkout them.

3) check out the branch locally.
We can add other remotes, to any of the forks of the repo (our personal forks), and checkout them. Same way as with upstream and/or origin remotes.

Please consider these ideas a just a few quick notes.

(I guess this small topic might be improved and wiki-worthy?)

In any case, any means are fine, the code is there to play around with, to improve, to transform, to suit your expectation and need. We will know when the "refactoring madness" is to (sort of) end, but until then we can have fun with it: there are a few which we wanted to cover before going beta (and we're getting there), apart from which it's simply what we get to.
The best moment for testing your PR is right after you merge it. Can't miss with that one.

Re: Code reviews notes

Reply #1

Quote from: TestMonkey – While most are localized (somewhat small), Ema's are wide :D
 emanuele takes competitions seriously! :P

Quote from: TestMonkey – Regardless how localized, the overall result is: they improve significantly the codebase we later work with.
 emanuele is not so sure about his refactoring... lol

Quote from: TestMonkey – And there's a word which comes with them: code reviews! :)
The more, the better. You never know what bit, small or big, another pair of eyes on the code will notice. Even questions or discussion on the code, have an extra chance to make it better, to spin off in a cleaner solution.
Exactly the reason I send PRs (to get feedback). So don't be shy and tell me if something is just plain useless (or ugly...or both :P)! ;D
Bugs creator.
Features destroyer.
Template killer.