[Swan-dev] [Swan-commit] Changes to ref refs/heads/master

[Tuomo Soini]

On Sat, 29 Mar 2014 15:02:36 -0400
"Lennart Sorensen" <lsorense at csclub.uwaterloo.ca> wrote:

> On Sat, Mar 29, 2014 at 01:54:50PM -0400, Paul Wouters wrote:
> > On Sat, 29 Mar 2014, Tuomo Soini wrote:
> > 
> > >Uncrustify did wrong in these cases. It was supposed to use tabs
> > >only but it used tabs and spaces. First one is wrong if it
> > >includes use of spaces for indent. Tabs should be used and match
> > >as near as possible with tabs. And no, I'm not perfect either with
> > >these. But those I tried to fix included spaces for indent which
> > >is wrong.
> > 
> > So I used tabs+spaces because I thought that's what the new style
> > was. I thought it pretty annoying so I'm happy to stop doing so and
> > stick with just tabs.
> 
> I thought it was decided to use Linux kernel style, but maybe I am
> thinking of a different discussion.

Yes, kernel coding style was selected and should be used. Problem is
the uncrustify tool we used to do the heavy lifting left some mess
behind because we didn't check all 200+ options.
> 
> Certainly the kernel seems to use tabs and a few spaces (if needed)
> to make function declarations line up, such as:

No. coding style is very strict about not to use spaces for indent. You
are expected to match as near as you can with tabs, not match exactly
like in your sample.
> 
> void ext4_journal_abort_handle(const char *caller, unsigned int line,
> 			       const char *err_fn, struct buffer_head
> *bh, handle_t *handle, int err)
> {
> 
> For functions calls, the continuation seems to be just indented with
> one extra tab, but in other cases (like here), it uses the same tab +
> up to 7 spaces to line up.
> 
> 	printk(KERN_ERR "EXT4-fs: %s:%d: aborting transaction: %s in
> %s\n", caller, line, errstr, err_fn);
> 
> 	jbd2_journal_abort_handle(handle);
> 
> And tabs are always assumed to be 8 characters.  If you set your
> editor to something else, then it is your problem when things don't
> align. :)

Very true.

> After all if you just cat the file, then it will look right unless
> your terminal is fucked up.

My commits I have always tested with kernel provided checkpatch.pl
utility to conform coding style. I guess everybody should use the
utility for now before committing stuff in so we all learn our coding
style. It's not easy process to switch coding style when we all know
better than coding style which is correct style :-)

Our worst problem is too long functions - we have over 1000 lines long
functions in the code and we should start splitting those to more
sensible size, maximum should be like 50 lines of code for each
function - but that's hard and time consuming work. Only way to do that
is piece by piece so we have working code all the time and won't break
it with too big changes at the same time.

-- 
Tuomo Soini <tis at foobar.fi>
Foobar Linux services
+358 40 5240030
Foobar Oy <http://foobar.fi/>