ElkArte Community

General => Chit Chat => Topic started by: TestMonkey on March 06, 2013, 02:09:16 am

Title: Code reviews notes
Post by: TestMonkey on March 06, 2013, 02:09:16 am
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.
Title: Re: Code reviews notes
Post by: emanuele on March 06, 2013, 07:52:47 am
Quote from: TestMonkey – While most are localized (somewhat small), Ema's are wide :D
/me takes competitions seriously! :P

Quote from: TestMonkey – Regardless how localized, the overall result is: they improve significantly the codebase we later work with.
/me 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