Re: [Tracker] Another Solaris patch

[Martyn Russell <martyn lanedo com>]

On 11/07/13 10:24, Philip Van Hoof wrote:
> Hi Ralph,

Hi,

> I have applied this patch to master. Two notes for future.

Thanks Philip for doing this, you beat me to it :P

> ps. I wouldn't make these notes just for a single patch as yours, but as
> I noticed there's an interest in publishing and pushing multiple such
> patches I'll make the recommendations now:
>
> o. Get the indentation right. Your patch used spaces for indentation. We
> use tabs for indentation and spaces for alignment as described here:
> http://pvanhoof.be/blog/index.php/2009/09/25/indentation

My job is done if Philip inherits this role :D

> If you encounter old code that is wrongly indented, adapt the code
> surrounding your patch to match those indentation rules (but don't adapt
> the entire file, only the stuff relevant to your changes, as in the end

This is a hard lesson. It pushes my buttons too. I always want to fix 
the whole file ... but it makes long term maintenance harder. Plus, I 
have a script or two to blitz the whole repository and fix these things 
:D save your time. We do this rarely.

> we still care more about git bisect working in a usable way than we care
> about indentation nazism. Although Martyn, bless our great release
> maintainer, can be a real pain in the ass when it comes to getting your
> indentation right. Don't say I didn't warn you ;-).

You're too kind :)

> I'm not sure if he's right or not for being so hard on this, but in the
> end you'll have to yield and do it right or it just wont get in. Hehe.

Just stopping eyes from bleeding.
Readability is more important to those maintaining admittedly.
The easier it is to read the more time one has for other things.

> Someday we'll be big and strong like Martyn, bless our great release
> maintainer, and then you can enforce arbitrary code indentation rules
> yourself. We love you Martyn.
>
> o. Put the component that you committed to upfront the commit message.
> In this case you could have used 'libtracker-common: what you changed'.

Yea, this helps when we release, means less work for release maintainer 
(me or Jürg) and more details for people reading.

> (Always) work in a branch, do git rebase -i master (or origin) to rebase
> your branch commits on top of master (and merge them, indeed), and then
> git push rebasedbranch:master or do git checkout master; git merge
> rebasedbranch; git push origin master. That or just publish to us your
> rebasedbranch (on github for example) and then we'll push it.

Working in branches really helps too keep your work in discrete parts so 
we can review it and merge it onto multiple branches more easily too.

Thanks Philip,

--
Regards,
Martyn

Founder and CEO of Lanedo GmbH.