[U-Boot-Users] Enforcement of coding standards

[U-Boot-Users] Enforcement of coding standards

[Chris Elston]

I don't really want to get into the politics of when patches should be
accepted/rejected, but I do agree that we need to have an honest (and
friendly!) discussion of the #ifdef mess and coding standard enforcement
issues.

Both Robert and Wolfgang have fair points.  From Robert's point of view why
be picky about formatting when the rest of the source isn't as neat as it
could be.  And from Wolfgang's point of view, why add more messy code - that
will just make things worse.

Maybe we could have a blitz on everything where we just check and fix
adherence to the coding standards - no functionality changes, just
readability.  Once we have the codebase in a 'tidied' state then Wolfgang
can more justifiably reject patches if they don't meet the standards.  

I think we can all agree that in places the source is a little untidy, and
that we wish to aim towards as readable and clear tree as we can - so let's
pull together and sort it out!

Thanks

Chris.

> -----Original Message-----
> From: Robert Schwebel [mailto:r.schwebel at pengutronix.de]
> Sent: 06 March 2003 16:08
> To: U-Boot Mailing List
> Subject: Re: [U-Boot-Users] [PATCH] 2/9: bootp
> 
> 
> On Thu, Mar 06, 2003 at 04:31:10PM +0100, Wolfgang Denk wrote:
> > Robert, what do you want to demonstrate?
> > 
> > That U-Boot was not written by one single person, stickng 
> to  exactly
> > one  coding  style?  That  there  are  deficiencies,  both 
> formal and
> > functional?
> > 
> > What you do right now is not helpful. You have enough  
> experience  to
> > provide  really  valuable  input - see some of your 
> previous patches.
> > Please try to focus on substantial things, and concentrate  
> on  fixes
> > and extensions.
> 
> You want open words, ok, here we go. 
> 
> Don't get me wrong, I generally have no problem with maintainers
> rejecting my patches.  It's quite normal that maintainers know their
> projects much better than I do, so I'm used to going back to 
> the lab and
> reworking stuff when it's necessary. 
> 
> My problem is that your argumentation regarding the "little things" is
> not easily understandable. You have a document in your code which says
> that Linux coding style should be used. If I send patches which fix
> coding style (and yes, it's only in source files I have worked on,
> otherwhise I wouldn't have found it) they are rejected. You 
> say: improve
> documentation; if I find something and do it you reject it 
> because it is
> not _exactly_ how you would have done it or how you did it. I try to
> improve usability by making help messages more understandable, because
> I, when I first tried to _use_ them didn't understand them and had to
> look at the source first (every good engineer should know how 
> important
> the grandma test is ;). You reject them because I add 10 
> bytes to a 100
> KiB bootloader. I try to improve #ifdef mess (and there's a lot of it
> left, I can tell you!) by using all the well known techniques 
> like debug
> macros etc. You reject them because it doesn't change functionality. I
> try to make code better readable by unsing correct indentation - you
> reject it. Then, after all that 'it-doesn't-matter-how-the-code-looks-
> like-if-it-works' I add two lines with
> 
> //#define foo
> #undef foo
>  
> and you tell me that it's against the coding style. My impression is
> that you didn't care a single bit about coding style with the other 
> 3.2 MiB of the code, so why do you care about my little improvements?
> It's not that easy to understand.  
> 
> Wolfgang, all these puzzle pieces are not worth to be 
> mentioned when you
> see them separately, and I definitely have better things to do than
> starting flame wars. But all that stuff together - including your
> sometimes a little bit rude RTFM postings addressed to people who are
> _not_ as deep into the project as you are - definitely don't 
> improve the
> mood of the developers here. 
> 
> Enough said - I would love to see an open discussion about how to
> improve the coding style / #ifdef problems. 
> 
> Robert 
> -- 
>  Dipl.-Ing. Robert Schwebel | http://www.pengutronix.de
>  Pengutronix - Linux Solutions for Science and Industry
>    Braunschweiger Str. 79,  31134 Hildesheim, Germany
>    Handelsregister:  Amtsgericht Hildesheim, HRA 2686
>     Phone: +49-5121-28619-0 |  Fax: +49-5121-28619-4
> 
> 
> -------------------------------------------------------
> This SF.net email is sponsored by: Etnus, makers of 
> TotalView, The debugger 
> for complex code. Debugging C/C++ programs can leave you 
> feeling lost and 
> disoriented. TotalView can help you find your way. Available 
> on major UNIX 
> and Linux platforms. Try it free. www.etnus.com
> _______________________________________________
> U-Boot-Users mailing list
> U-Boot-Users at lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/u-boot-users
> 
> ______________________________________________________________
> __________
> This e-mail has been scanned for all viruses by Star Internet. The
> service is powered by MessageLabs. For more information on a proactive
> anti-virus service working around the clock, around the globe, visit:
> http://www.star.net.uk
> ______________________________________________________________
> __________
> 

________________________________________________________________________
This e-mail has been scanned for all viruses by Star Internet. The
service is powered by MessageLabs. For more information on a proactive
anti-virus service working around the clock, around the globe, visit:
http://www.star.net.uk
________________________________________________________________________

[U-Boot-Users] Enforcement of coding standards my $0.02 worth

[Rod Boyce]

All,

After having read a lot of this thread.  I'm going to add my $0.02 worth
to this discussion.  'ident' is your friend it can convert the source
very easily to any format you would like.  I sometime agree Wolfgang can
be a but picky but without a doubt I believe that Wolfgang is doing a
great job of keeping a now very multi-platform boot loader that has to
deal with many different CPU's and sometime conflicting hardware
requirements a cohesive unity that is easy to port to different
platforms. I agree with Wolfgang we all are standing on the shoulders of
those who have gone before us. 

IMHO the code is untidy in quite a few places.  I have noticed Wolfgang
has reformatted patches I have sent him but who cares.  I say Wolfgang
is doing a fantastic job and to keep up the good work.  I myself and
many others on this list owe Wolfgang a beer or two if I ever get to
meet you in person.

Regards,
Rod Boyce.

PS I have disagreed with Wolfgang in the past but I still believe that
U-Boot is a better product because Wolfgang is the lead and the
maintainer a thankless job on many ocasions.

On Thu, 6 Mar 2003 16:16:30 -0000
Chris Elston <elston@radstone.co.uk> wrote:

> I don't really want to get into the politics of when patches should be
> accepted/rejected, but I do agree that we need to have an honest (and
> friendly!) discussion of the #ifdef mess and coding standard enforcement
> issues.
> 
> Both Robert and Wolfgang have fair points.  From Robert's point of view why
> be picky about formatting when the rest of the source isn't as neat as it
> could be.  And from Wolfgang's point of view, why add more messy code - that
> will just make things worse.
> 
> Maybe we could have a blitz on everything where we just check and fix
> adherence to the coding standards - no functionality changes, just
> readability.  Once we have the codebase in a 'tidied' state then Wolfgang
> can more justifiably reject patches if they don't meet the standards.  
> 
> I think we can all agree that in places the source is a little untidy, and
> that we wish to aim towards as readable and clear tree as we can - so let's
> pull together and sort it out!
> 
> Thanks
> 
> Chris.
> 
> > -----Original Message-----
> > From: Robert Schwebel [mailto:r.schwebel at pengutronix.de]
> > Sent: 06 March 2003 16:08
> > To: U-Boot Mailing List
> > Subject: Re: [U-Boot-Users] [PATCH] 2/9: bootp
> > 
> > 
> > On Thu, Mar 06, 2003 at 04:31:10PM +0100, Wolfgang Denk wrote:
> > > Robert, what do you want to demonstrate?
> > > 
> > > That U-Boot was not written by one single person, stickng 
> > to  exactly
> > > one  coding  style?  That  there  are  deficiencies,  both 
> > formal and
> > > functional?
> > > 
> > > What you do right now is not helpful. You have enough  
> > experience  to
> > > provide  really  valuable  input - see some of your 
> > previous patches.
> > > Please try to focus on substantial things, and concentrate  
> > on  fixes
> > > and extensions.
> > 
> > You want open words, ok, here we go. 
> > 
> > Don't get me wrong, I generally have no problem with maintainers
> > rejecting my patches.  It's quite normal that maintainers know their
> > projects much better than I do, so I'm used to going back to 
> > the lab and
> > reworking stuff when it's necessary. 
> > 
> > My problem is that your argumentation regarding the "little things" is
> > not easily understandable. You have a document in your code which says
> > that Linux coding style should be used. If I send patches which fix
> > coding style (and yes, it's only in source files I have worked on,
> > otherwhise I wouldn't have found it) they are rejected. You 
> > say: improve
> > documentation; if I find something and do it you reject it 
> > because it is
> > not _exactly_ how you would have done it or how you did it. I try to
> > improve usability by making help messages more understandable, because
> > I, when I first tried to _use_ them didn't understand them and had to
> > look at the source first (every good engineer should know how 
> > important
> > the grandma test is ;). You reject them because I add 10 
> > bytes to a 100
> > KiB bootloader. I try to improve #ifdef mess (and there's a lot of it
> > left, I can tell you!) by using all the well known techniques 
> > like debug
> > macros etc. You reject them because it doesn't change functionality. I
> > try to make code better readable by unsing correct indentation - you
> > reject it. Then, after all that 'it-doesn't-matter-how-the-code-looks-
> > like-if-it-works' I add two lines with
> > 
> > //#define foo
> > #undef foo
> >  
> > and you tell me that it's against the coding style. My impression is
> > that you didn't care a single bit about coding style with the other 
> > 3.2 MiB of the code, so why do you care about my little improvements?
> > It's not that easy to understand.  
> > 
> > Wolfgang, all these puzzle pieces are not worth to be 
> > mentioned when you
> > see them separately, and I definitely have better things to do than
> > starting flame wars. But all that stuff together - including your
> > sometimes a little bit rude RTFM postings addressed to people who are
> > _not_ as deep into the project as you are - definitely don't 
> > improve the
> > mood of the developers here. 
> > 
> > Enough said - I would love to see an open discussion about how to
> > improve the coding style / #ifdef problems. 
> > 
> > Robert 
> > -- 
> >  Dipl.-Ing. Robert Schwebel | http://www.pengutronix.de
> >  Pengutronix - Linux Solutions for Science and Industry
> >    Braunschweiger Str. 79,  31134 Hildesheim, Germany
> >    Handelsregister:  Amtsgericht Hildesheim, HRA 2686
> >     Phone: +49-5121-28619-0 |  Fax: +49-5121-28619-4
> > 
> > 
> > -------------------------------------------------------
> > This SF.net email is sponsored by: Etnus, makers of 
> > TotalView, The debugger 
> > for complex code. Debugging C/C++ programs can leave you 
> > feeling lost and 
> > disoriented. TotalView can help you find your way. Available 
> > on major UNIX 
> > and Linux platforms. Try it free. www.etnus.com
> > _______________________________________________
> > U-Boot-Users mailing list
> > U-Boot-Users at lists.sourceforge.net
> > https://lists.sourceforge.net/lists/listinfo/u-boot-users
> > 
> > ______________________________________________________________
> > __________
> > This e-mail has been scanned for all viruses by Star Internet. The
> > service is powered by MessageLabs. For more information on a proactive
> > anti-virus service working around the clock, around the globe, visit:
> > http://www.star.net.uk
> > ______________________________________________________________
> > __________
> > 
> 
> ________________________________________________________________________
> This e-mail has been scanned for all viruses by Star Internet. The
> service is powered by MessageLabs. For more information on a proactive
> anti-virus service working around the clock, around the globe, visit:
> http://www.star.net.uk
> ________________________________________________________________________
> 
> 
> -------------------------------------------------------
> This SF.net email is sponsored by: Etnus, makers of TotalView, The debugger 
> for complex code. Debugging C/C++ programs can leave you feeling lost and 
> disoriented. TotalView can help you find your way. Available on major UNIX 
> and Linux platforms. Try it free. www.etnus.com
> _______________________________________________
> U-Boot-Users mailing list
> U-Boot-Users at lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/u-boot-users

[U-Boot-Users] Enforcement of coding standards

[Robert Schwebel]

On Thu, Mar 06, 2003 at 04:16:30PM -0000, Chris Elston wrote:
> I don't really want to get into the politics of when patches should be
> accepted/rejected, but I do agree that we need to have an honest (and
> friendly!) discussion of the #ifdef mess and coding standard enforcement
> issues.

Strong ack. 

> Both Robert and Wolfgang have fair points.  From Robert's point of view why
> be picky about formatting when the rest of the source isn't as neat as it
> could be.  And from Wolfgang's point of view, why add more messy code - that
> will just make things worse.

No, the point was that these two // have been rejected, after lots of
coding style patches I've sent have also been rejected because "they do
not change functionality". Just for the record, I wanted to shut up
about that :-) 

Robert 
-- 
 Dipl.-Ing. Robert Schwebel | http://www.pengutronix.de
 Pengutronix - Linux Solutions for Science and Industry
   Braunschweiger Str. 79,  31134 Hildesheim, Germany
   Handelsregister:  Amtsgericht Hildesheim, HRA 2686
    Phone: +49-5121-28619-0 |  Fax: +49-5121-28619-4

[U-Boot-Users] Enforcement of coding standards

[Wolfgang Denk]

In message <20030307062951.GD16290@pengutronix.de> you wrote:
>
> No, the point was that these two // have been rejected, after lots of
> coding style patches I've sent have also been rejected because "they do
> not change functionality". Just for the record, I wanted to shut up
> about that :-) 

Actually (from my point of view) there is some consistency  in  this:
if  functionality is not effected, I usually don't want to change the
code, not to the better and of course not to the worse either.

This is pretty often a selfish decision - please  try  to  understand
what I have to do when merging patches from many different developers
to maintain a certain degree of stability and reliability.

To understand a modification, or possible interactions between  unre-
lated modifications, I often run diff's over the source tree.

I try to actually run a certain set of regression tests (at least all
the examples in the DPLG document - these are generated automagically
when I run my test suite) at least on one board  for  each  processor
family,  plus  on  all  boards  for  which there is kind of a support
contract.

If there is any problem or change in behaviour I  find  myself  very,
very often to run diff's between versions.

Obviously, I am interested  in  keeping  these  diff's  as  small  as
possible.  Running  "cb"  or "indent" over a source file makes a diff
against earlier versions basicly useless.


When I resist to a global reformatting of all sources  this  is  pure
self-defense. I really _need_ the history in the source files.

Best regards,

Wolfgang Denk

-- 
Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-4596-87  Fax: (+49)-8142-4596-88  Email: wd at denx.de
There are some things worth dying for.
	-- Kirk, "Errand of Mercy", stardate 3201.7

[U-Boot-Users] Enforcement of coding standards

[Robert Schwebel]

On Fri, Mar 07, 2003 at 09:11:58AM +0100, Wolfgang Denk wrote:
> Obviously, I am interested in keeping these diff's as small as
> possible. Running "cb" or "indent" over a source file makes a diff
> against earlier versions basicly useless.

Understandable - would it help if we had something like a stable and
unstable branch? 

I mean, the problem is that for the short time you are perfectly right
with your arguments. But long term some parts of the code definitely
have to be cleaned up or everybody will be lost. 

For the moment I don't really care about the huge rest of the code; the
things I'm working on are currently mostly PXA related and I consider
the PXA port still being code under construction.

Perhaps some kind of a release plan would be helpful for the project,
now that the code seems to work for several people. One could make
checklists for a release, something like 

	board 	compiles	tested	maintainer	tested-by ... 
	foo	x		x	Fridolin Tux	Erich Tux
	...

This would mean to push the QA from CVS to release/prerelease level. 

Robert
-- 
 Dipl.-Ing. Robert Schwebel | http://www.pengutronix.de
 Pengutronix - Linux Solutions for Science and Industry
   Braunschweiger Str. 79,  31134 Hildesheim, Germany
   Handelsregister:  Amtsgericht Hildesheim, HRA 2686
    Phone: +49-5121-28619-0 |  Fax: +49-5121-28619-4

[U-Boot-Users] Enforcement of coding standards

[Wolfgang Denk]

In message <20030307083407.GN16290@pengutronix.de> you wrote:
>
> Understandable - would it help if we had something like a stable and
> unstable branch? 

I tried this twice (internally). It does not help. I prefer  to  have
one tree, and to stabilize it for each "release".

> I mean, the problem is that for the short time you are perfectly right
> with your arguments. But long term some parts of the code definitely
> have to be cleaned up or everybody will be lost. 

Well, yes and no. Yes, you are right, and it is often  pretty  simple
for  isolated  code.  Cleaning up one ethernet driver or the code for
one specific RTC will not cause many problems. But  don't  touch  the
init  sequence  rashly,  or bigger sub-systems like I2C or PCI. There
are LOTS of things to consider.

> For the moment I don't really care about the huge rest of the code; the
> things I'm working on are currently mostly PXA related and I consider
> the PXA port still being code under construction.

Indeed. Just go on there.

> Perhaps some kind of a release plan would be helpful for the project,
> now that the code seems to work for several people. One could make
> checklists for a release, something like 
> 
> 	board 	compiles	tested	maintainer	tested-by ... 
> 	foo	x		x	Fridolin Tux	Erich Tux
> 	...

Fine. We can save the  "compiles"  column  because  usually  I  don't
accept any changes that don't compile.

But then - look at the MAINTAINERS file for the long list of orphaned
boards. What do we do with those?

Best regards,

Wolfgang Denk

-- 
Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-4596-87  Fax: (+49)-8142-4596-88  Email: wd at denx.de
A father doesn't destroy his children.
	-- Lt. Carolyn Palamas, "Who Mourns for Adonais?",
	   stardate 3468.1.

[U-Boot-Users] Enforcement of coding standards

[Robert Schwebel]

On Fri, Mar 07, 2003 at 09:58:31AM +0100, Wolfgang Denk wrote:
> But then - look at the MAINTAINERS file for the long list of orphaned
> boards. What do we do with those?

Hmm, if nobody maintains a port any more he cannot hope that it will
work on forever. From time to time this has to be checked. Surely,
nobody will break other people's boards by intention, but on the other
hand nobody can expect that changes don't affect a board over a longer
development time, especially when large changes like the u-boot merge
are done.  

Robert
-- 
 Dipl.-Ing. Robert Schwebel | http://www.pengutronix.de
 Pengutronix - Linux Solutions for Science and Industry
   Braunschweiger Str. 79,  31134 Hildesheim, Germany
   Handelsregister:  Amtsgericht Hildesheim, HRA 2686
    Phone: +49-5121-28619-0 |  Fax: +49-5121-28619-4