Hi Hendrik,
On Sat, Jul 03, 2010, Hendrik Sattler wrote:
> All my changes since commit 3b293983338985d2d0f43451dcc4e07a91506c97:
>
> Remove alread removed function from documentation (2010-03-01 14:57:08
> -0300)
>
> are available in the git repository at:
> git://gitorious.org/openobex/mainline.git updates
>
> The changelog is not here at purpose, please see the chronological one at
> http://www.gitorious.org/openobex/mainline/commits/updates
> instead.
Your tree seems to contain lots of good stuff. Any reason why you
haven't requested an upstream merge earlier? The changes span from early
march to the end of june so it seems this would have been possible a
long time ago. Now the change set is quite huge and will take a long
time to properly review. After a quick skim though I didn't find
anything really critical, but there seem to be plenty of white space
issues that'd be nice to get fixed. Also, please keep your commit
message width at max 72 characters so they're readable with git log on
80 character terminals.
The whitespace issues I noticed fall mainly into the following
categories:
- over 80-character line (ok if the existing code had that issue too)
- white space at the end of line before line terminator
- mixed tabs and spaces for indentation
- more than one consecutive empty line
- some empty lines not being really empty but containing tabs or spaces
- space between function name and opening (
At least when I run "git log -p origin.." it is able to visually show
many of these violations. I suspect that it's thanks to the following in
my .gitconfig:
[diff]
color = auto
I also have the following in my .vimrc that shows most issues:
highlight RedundantWhitespace ctermbg=red guibg=red
match RedundantWhitespace /\s\+$\|\t\+\ze \+[^*]\| \+\ze\t/
If you're using vim or have it available maybe you could try that too?
I understand that it can be tricky to start fixing old commits when you
have so many new ones that might depend on their content (causing
potential conflicts if you go changing stuff), so if it gets too
complicated I suppose having coding-style fixup commits on top of the
existing changes is the only option (we had this situation some time
with obexd when INdT's own trees changes grew too big). However, if at
all possible, try to fix the existing commits in place.
Also, it'd be good to avoid commits that fix regressions in other
non-upstream commits. The last one, "Fix libusb-0.x transport, broke
with b5b64ffc", is one of those. In that case I think it'd be better to
just redo commit b5b64ffc not to introduce the regression.
Johan
------------------------------------------------------------------------------
This SF.net email is sponsored by Sprint
What will you do first with EVO, the first 4G phone?
Visit sprint.com/first -- http://p.sf.net/sfu/sprint-com-first
_______________________________________________
Openobex-users mailing list
Openobex-users@lists.sourceforge.net
http://lists.sourceforge.net/lists/listinfo/openobex-users
