You are viewing a plain text version of this content. The canonical link for it is here.
Posted to derby-dev@db.apache.org by Kev Jackson <ke...@it.fts-vn.com> on 2006/02/07 03:28:12 UTC
[patch] cleanup of org.apache.derby.iapi.services.cache.ClassSize
Following on from yesterdays post, here's a patch that tries to get this
class to conform to the coding standards chosen by the Derby developers
- remove unused import
- change static final variables from camelCase to CONST_NAMES (except
where they are public and could be used elsewhere BWC concern)
- add {} for conditionals
- strip lots of extra white space
As for the CLA/ICLA, I've already signed one (for ant), so do I need to
sign another for Derby?
Thanks
Kev
Re: [patch] cleanup of org.apache.derby.iapi.services.cache.ClassSize
Posted by Daniel John Debrunner <dj...@apache.org>.
Kathey Marsden wrote:
> Shortly after the client contribution, Jeremy changed all the client
> files to have a uniform style.
Though I don't think there was any followup on how to ensure the new
format was kept to, or if patches with a different style (to what?)
should be accepted.
> I have recently been thinking that I would like to see the Network
> Server code on the trunk and 10.1 changed to match client, especially
> since many changes include client and server modifications and we may
> be able to share code in the future. We could pick some weekend in a
> few weeks and see if Jeremy would be willing to make a pass of the drda
> tree, which is not really that many files. We spend too much time
> messing with this issue. Thoughts?
Doesn't have to be Jeremy, could be anyone. If a non-committer does this
it might be better for them to provide a script that performs the
re-formatting rather than a big patch, easier to ensure there's no
problem with the changes.
Dan.
Re: [patch] cleanup of org.apache.derby.iapi.services.cache.ClassSize
Posted by Kathey Marsden <km...@sbcglobal.net>.
Kev Jackson wrote:
>
>> Right, and Derby has not chosen either of those styles, the text is
>> correct in saying *individuals* have found those guides helpful.
>>
>>
>
> Ah ok, so not that these are the only style guides that contributors
> should follow, I should read more carefully
>
Shortly after the client contribution, Jeremy changed all the client
files to have a uniform style.
I have recently been thinking that I would like to see the Network
Server code on the trunk and 10.1 changed to match client, especially
since many changes include client and server modifications and we may
be able to share code in the future. We could pick some weekend in a
few weeks and see if Jeremy would be willing to make a pass of the drda
tree, which is not really that many files. We spend too much time
messing with this issue. Thoughts?
Question.
Was client made to match the Geronimo style
http://wiki.apache.org/geronimo/CodingStandards? If so we can just
point to that for client and network server.
Kathey
P.S. I have no opinion about the other code.
Re: [patch] cleanup of org.apache.derby.iapi.services.cache.ClassSize
Posted by "David W. Van Couvering" <Da...@Sun.COM>.
I like the idea of a script run at build time that checks coding style.
I could get behind that. One thing you could do to help, Kev, since
this is your itch, is to (a) propose a coding style and (b) propose a
process by which this style can be adhered to that is not too onerous
such that it inhibits contributions or significantly impacts the
productivity of developers.
Thanks,
David
Kev Jackson wrote:
>
>> Right, and Derby has not chosen either of those styles, the text is
>> correct in saying *individuals* have found those guides helpful.
>>
>>
>
> Ah ok, so not that these are the only style guides that contributors
> should follow, I should read more carefully
>
>>
>>
>>> - also it's not good to have
>>> unused imports kicking around, it adds bloat to the code, and it's
>>> unnecessary, removing these wouldn't hurt regardless of the rest of the
>>> changes.
>>>
>>
>>
>> Agree there, removing unused imports is a good thing to do. Though they
>> don't do any harm to the runtime footprint, only the .java file and
>> cause yellow flags in Eclipse.
>>
>>
>>
> This is one of my concerns, Eclipse has a pretty good warning system,
> but if it's flooded with small silly warnings (things that should really
> be fixed), then it's possible to miss out on the warning that's
> important. At the moment there are 7000+ warnings (of all different
> types) reported by Eclipse - how many of these are really important, I
> don't know, but by getting rid of the easy ones it's easier to see the
> important ones where perhaps the warning should be taken seriously or
> perhaps not.
>
>>> Also I'm suprised that there isn't a defined style (regardless of
>>> whether I like it or not) - every single other open source project I've
>>> worked on (Java, C, Ruby, whatever) has had a style that submitted
>>> patches must conform to before they are considered for inclusion, and
>>> it's a little odd that Derby doesn't have this.
>>>
>>
>>
>> It's because no one has had a stong itch to enforce a coding style, and
>> come up with the rules/process for accepting a patch or not (do we want
>> to turn away contributions due to a coding style issue, do the
>> committers want to spend their time checking coding standards). Any
>> experience you want to bring from other projects would be great here.
>>
>>
> The only thing I would say here is that the projects I've worked on tend
> to be upfront about the coding style which is being enforced (Ant is the
> Sun coding conventions (roughly), Rails is 2 spaces, no tabs, Ruby style
> for method names, Ruby is follow the current code etc), for code that is
> already in svn/cvs, the most sensible thing would be to gradually change
> it as a committer comes across it, or during refactoring etc.
> As I mentioned before, it's better if there is an automated process to
> check files so that it's not seen as a personal issue (which it isn't).
> Jalopy, checkstyle, pmd are all good for checking Java and all can be
> run as part of the build process (all have Ant tasks). If a style is
> chosen, then supplying a template file for IDEA/Eclipse/vim/emacs would
> also be of benefit as then the developers can code how they like and the
> IDE can massage the code into "the one true style". An svn check in
> script could also be used to fix up formatting before the commit takes
> place.
>
> Obviously as a newcomer I don't want to rock the boat, (and style/brace
> wars are always nasty) - hope I didn't cause offense.
>
> Kev
Re: [patch] cleanup of org.apache.derby.iapi.services.cache.ClassSize
Posted by Kev Jackson <ke...@it.fts-vn.com>.
>Right, and Derby has not chosen either of those styles, the text is
>correct in saying *individuals* have found those guides helpful.
>
>
Ah ok, so not that these are the only style guides that contributors
should follow, I should read more carefully
>
>
>>- also it's not good to have
>>unused imports kicking around, it adds bloat to the code, and it's
>>unnecessary, removing these wouldn't hurt regardless of the rest of the
>>changes.
>>
>>
>
>Agree there, removing unused imports is a good thing to do. Though they
>don't do any harm to the runtime footprint, only the .java file and
>cause yellow flags in Eclipse.
>
>
>
This is one of my concerns, Eclipse has a pretty good warning system,
but if it's flooded with small silly warnings (things that should really
be fixed), then it's possible to miss out on the warning that's
important. At the moment there are 7000+ warnings (of all different
types) reported by Eclipse - how many of these are really important, I
don't know, but by getting rid of the easy ones it's easier to see the
important ones where perhaps the warning should be taken seriously or
perhaps not.
>>Also I'm suprised that there isn't a defined style (regardless of
>>whether I like it or not) - every single other open source project I've
>>worked on (Java, C, Ruby, whatever) has had a style that submitted
>>patches must conform to before they are considered for inclusion, and
>>it's a little odd that Derby doesn't have this.
>>
>>
>
>It's because no one has had a stong itch to enforce a coding style, and
>come up with the rules/process for accepting a patch or not (do we want
>to turn away contributions due to a coding style issue, do the
>committers want to spend their time checking coding standards). Any
>experience you want to bring from other projects would be great here.
>
>
The only thing I would say here is that the projects I've worked on tend
to be upfront about the coding style which is being enforced (Ant is the
Sun coding conventions (roughly), Rails is 2 spaces, no tabs, Ruby style
for method names, Ruby is follow the current code etc), for code that is
already in svn/cvs, the most sensible thing would be to gradually change
it as a committer comes across it, or during refactoring etc.
As I mentioned before, it's better if there is an automated process to
check files so that it's not seen as a personal issue (which it isn't).
Jalopy, checkstyle, pmd are all good for checking Java and all can be
run as part of the build process (all have Ant tasks). If a style is
chosen, then supplying a template file for IDEA/Eclipse/vim/emacs would
also be of benefit as then the developers can code how they like and the
IDE can massage the code into "the one true style". An svn check in
script could also be used to fix up formatting before the commit takes
place.
Obviously as a newcomer I don't want to rock the boat, (and style/brace
wars are always nasty) - hope I didn't cause offense.
Kev
Re: [patch] cleanup of org.apache.derby.iapi.services.cache.ClassSize
Posted by Daniel John Debrunner <dj...@apache.org>.
Kev Jackson wrote:
> Daniel John Debrunner wrote:
>
>> Kev Jackson wrote:
>>
>>
>>> Following on from yesterdays post, here's a patch that tries to get this
>>> class to conform to the coding standards chosen by the Derby developers
>>>
>>
>>
>> Except that we have not chosen a coding standard. :-)
>>
>> http://wiki.apache.org/db-derby/DerbyContributorChecklist
>>
>>
> In both coding standards listed (Java Code style from Sun and the
> Geronimo code style), this class does not conform to either style
Right, and Derby has not chosen either of those styles, the text is
correct in saying *individuals* have found those guides helpful.
> - also it's not good to have
> unused imports kicking around, it adds bloat to the code, and it's
> unnecessary, removing these wouldn't hurt regardless of the rest of the
> changes.
Agree there, removing unused imports is a good thing to do. Though they
don't do any harm to the runtime footprint, only the .java file and
cause yellow flags in Eclipse.
> Also I'm suprised that there isn't a defined style (regardless of
> whether I like it or not) - every single other open source project I've
> worked on (Java, C, Ruby, whatever) has had a style that submitted
> patches must conform to before they are considered for inclusion, and
> it's a little odd that Derby doesn't have this.
It's because no one has had a stong itch to enforce a coding style, and
come up with the rules/process for accepting a patch or not (do we want
to turn away contributions due to a coding style issue, do the
committers want to spend their time checking coding standards). Any
experience you want to bring from other projects would be great here.
Thanks,
Dan.
Re: [patch] cleanup of org.apache.derby.iapi.services.cache.ClassSize
Posted by Kev Jackson <ke...@it.fts-vn.com>.
Daniel John Debrunner wrote:
>Kev Jackson wrote:
>
>
>>Following on from yesterdays post, here's a patch that tries to get this
>>class to conform to the coding standards chosen by the Derby developers
>>
>>
>
>Except that we have not chosen a coding standard. :-)
>
>http://wiki.apache.org/db-derby/DerbyContributorChecklist
>
>
In both coding standards listed (Java Code style from Sun and the
Geronimo code style), this class does not conform to either style
>and in another e-mail:
>
>
>> I'm willing to contribute as I'd like to learn a bit about how databases work under the hood,
>>but I can't stand the current code style and I'd like to know if this is an agreed upon convention,
>>or a legacy of the various owners of the codebase up to now.
>>
>>
>
>The history of the code (from its inception over 8 years ago) was that
>the only coding standard in force was to be clear, and don't make
>changes in the code just for formatting sake. It was just not worth the
>time originally to try and get everyone to agree on a single standard,
>some people like braces one way, some the other, some like spaces in
>different positions in loops or if statements etc, sometimes a single
>rule is not applicable for all situations etc. etc.
>
>
>
if (condition)
do something;
is clear except when you see
if (condition)
return foo;
return bar;
(and an example of this can be found in the code that I changed (ie the
one in svn right now))
>The issue is that you say you can't stand the current code style, but
>maybe someone else can't stand a code style you like, hard to please
>everyone. :-)
>
>
>
Understood and a good way to compromise is through the enforcement of a
standard through something like checkstyle (so the machine is to blame,
not a person!) - but note that first I asked about style, then I was
directed to two (slightly different) styles linked to from the wiki and
this class didn't conform to either of them - hence I assumed that it
was worth refactoring a little to conform - also it's not good to have
unused imports kicking around, it adds bloat to the code, and it's
unnecessary, removing these wouldn't hurt regardless of the rest of the
changes.
Also I'm suprised that there isn't a defined style (regardless of
whether I like it or not) - every single other open source project I've
worked on (Java, C, Ruby, whatever) has had a style that submitted
patches must conform to before they are considered for inclusion, and
it's a little odd that Derby doesn't have this.
>Please do get involved in Derby, look at the jira issues marked with a
>component of Newcomer, or fix a bug thet scratches your itch with Derby.
>Many on the list will be willing to guide you, just ask.
>
>
>
>>- remove unused import
>>- change static final variables from camelCase to CONST_NAMES (except
>>where they are public and could be used elsewhere BWC concern)
>>
>>
>[just curious, what's BWC?]
>
>
BWC = BackWards Compatibility, this is #1 priority over at Ant, so I
assumed that no changes should be made to public values/methods to
prevent any BWC problems
>
>
>>- add {} for conditionals
>>- strip lots of extra white space
>>
>>
>
>As Myrna said, these type of changes can cause problems for merges of
>fixes across branches and also to others with existing edits against the
>same files.
>
>
>
>>As for the CLA/ICLA, I've already signed one (for ant), so do I need to
>>sign another for Derby?
>>
>>
>
>Nope, I belive a single ICLA at the ASF is sufficient.
>
>
Ok, well that's one question answered :)
Kev
Re: [patch] cleanup of org.apache.derby.iapi.services.cache.ClassSize
Posted by Andreas Korneliussen <An...@Sun.COM>.
Daniel John Debrunner wrote:
> Kev Jackson wrote:
>
>>Following on from yesterdays post, here's a patch that tries to get this
>>class to conform to the coding standards chosen by the Derby developers
>
>
> Except that we have not chosen a coding standard. :-)
I just read the following on http://db.apache.org/source.html :
-
All Java Language source code in the repository must be written in
conformance to the " Code Conventions for the Java Programming Language
as published by Sun, or in conformance with another well-defined
convention specified by the subproject. See the FAQ page for links to
subproject conventions.
-
The link to the FAQ page was dead.
I guess that if the subproject has not defined a coding standard, it
should use the standard of the "super" project ?
Andreas
>
> http://wiki.apache.org/db-derby/DerbyContributorChecklist
>
> and in another e-mail:
>
>> I'm willing to contribute as I'd like to learn a bit about how databases work under the hood,
>>but I can't stand the current code style and I'd like to know if this is an agreed upon convention,
>>or a legacy of the various owners of the codebase up to now.
>
>
> The history of the code (from its inception over 8 years ago) was that
> the only coding standard in force was to be clear, and don't make
> changes in the code just for formatting sake. It was just not worth the
> time originally to try and get everyone to agree on a single standard,
> some people like braces one way, some the other, some like spaces in
> different positions in loops or if statements etc, sometimes a single
> rule is not applicable for all situations etc. etc.
>
> The issue is that you say you can't stand the current code style, but
> maybe someone else can't stand a code style you like, hard to please
> everyone. :-)
>
> Please do get involved in Derby, look at the jira issues marked with a
> component of Newcomer, or fix a bug thet scratches your itch with Derby.
> Many on the list will be willing to guide you, just ask.
>
>
>>- remove unused import
>>- change static final variables from camelCase to CONST_NAMES (except
>>where they are public and could be used elsewhere BWC concern)
>
> [just curious, what's BWC?]
>
>
>>- add {} for conditionals
>>- strip lots of extra white space
>
>
> As Myrna said, these type of changes can cause problems for merges of
> fixes across branches and also to others with existing edits against the
> same files.
>
>
>>As for the CLA/ICLA, I've already signed one (for ant), so do I need to
>>sign another for Derby?
>
>
> Nope, I belive a single ICLA at the ASF is sufficient.
>
> Thanks,
> Dan.
>
Re: [patch] cleanup of org.apache.derby.iapi.services.cache.ClassSize
Posted by Daniel John Debrunner <dj...@apache.org>.
Kev Jackson wrote:
> Following on from yesterdays post, here's a patch that tries to get this
> class to conform to the coding standards chosen by the Derby developers
Except that we have not chosen a coding standard. :-)
http://wiki.apache.org/db-derby/DerbyContributorChecklist
and in another e-mail:
> I'm willing to contribute as I'd like to learn a bit about how databases work under the hood,
> but I can't stand the current code style and I'd like to know if this is an agreed upon convention,
> or a legacy of the various owners of the codebase up to now.
The history of the code (from its inception over 8 years ago) was that
the only coding standard in force was to be clear, and don't make
changes in the code just for formatting sake. It was just not worth the
time originally to try and get everyone to agree on a single standard,
some people like braces one way, some the other, some like spaces in
different positions in loops or if statements etc, sometimes a single
rule is not applicable for all situations etc. etc.
The issue is that you say you can't stand the current code style, but
maybe someone else can't stand a code style you like, hard to please
everyone. :-)
Please do get involved in Derby, look at the jira issues marked with a
component of Newcomer, or fix a bug thet scratches your itch with Derby.
Many on the list will be willing to guide you, just ask.
> - remove unused import
> - change static final variables from camelCase to CONST_NAMES (except
> where they are public and could be used elsewhere BWC concern)
[just curious, what's BWC?]
> - add {} for conditionals
> - strip lots of extra white space
As Myrna said, these type of changes can cause problems for merges of
fixes across branches and also to others with existing edits against the
same files.
> As for the CLA/ICLA, I've already signed one (for ant), so do I need to
> sign another for Derby?
Nope, I belive a single ICLA at the ASF is sufficient.
Thanks,
Dan.