You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@poi.apache.org by "Allison, Timothy B." <ta...@mitre.org> on 2015/05/28 21:36:26 UTC
style guidelines?
Hi All,
As ongoing penance for my regrettable past (really it is behind me...I hope...), I'd like to start cleaning up some of the code whenever I come across style issues. I'll follow the guidance of fixing whitespace in separate patches from content patches.
I found this:
https://poi.apache.org/resolutions/res001.html
and this:
https://poi.apache.org/guidelines.html#CodeStyle
Basic questions:
1) spaces are we aiming for four spaces?
2) brace spacing, I'm assuming ) { ..with one space and not put into next line.
3) new lines: lf
4) don't use wildcards on imports
5) Import order:
static
NEWLINE
java
NEWLINE
javax
NEWLINE
Everything else
This about right? Other issues/points?
Cheers,
Tim
P.S. Nick, thank you for your recent superb "welcome to developing with POI" email.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
Re: style guidelines?
Posted by Nick Burch <ap...@gagravarr.org>.
On Thu, 28 May 2015, Allison, Timothy B. wrote:
> As ongoing penance for my regrettable past (really it is behind me...I
> hope...), I'd like to start cleaning up some of the code whenever I come
> across style issues. I'll follow the guidance of fixing whitespace in
> separate patches from content patches.
>
> I found this:
> https://poi.apache.org/resolutions/res001.html
That's 13 years old now, so much of it has been replaced / overriden. We
probably want to make that clear at the top? Patches / commits for that
welcome!
> and this:
> https://poi.apache.org/guidelines.html#CodeStyle
That's the one we should be all using now. If there's any gaps in what it
says, we should agree + add them!
> Basic questions:
> 1) spaces are we aiming for four spaces?
> 2) brace spacing, I'm assuming ) { ..with one space and not put into next line.
> 3) new lines: lf
> 4) don't use wildcards on imports
>
> 5) Import order:
> static
> NEWLINE
> java
> NEWLINE
> javax
> NEWLINE
> Everything else
Probably... Looks roughly right at a first glance... Best check
https://poi.apache.org/guidelines.html#CodeStyle and flag up any not
clear! :)
> This about right? Other issues/points?
I believe that the eclipse project settings should be getting most of
these for eclipse users. If we spot anything wrong / agree any changes, we
should update them. If an Idea user wants to contribute some matching
project files for that, they'd be great too, provided they match!
Nick
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
RE: style guidelines?
Posted by "Allison, Timothy B." <ta...@mitre.org>.
Too late to be useful, but sounds great. Thank you!
P.S. I found that Intellij's autocorrect-style function broke XWPFRelation because it moved the initialization of _table to after the initialization of its contents...leading to an NPE...ha!
-----Original Message-----
From: Dominik Stadler [mailto:dominik.stadler@gmx.at]
Sent: Saturday, May 30, 2015 7:20 AM
To: POI Developers List
Subject: Re: style guidelines?
Hi,
I though about performing an automatic cleanup via Eclipse tooling at
some point, this would fix some of the inconsistencies across the
whole source and should provide a bit more consistency, albeit a full
automated reformatting is probably too much as it would make some code
less readable.
A big downside of this is that patches done with previous version
likely will not apply cleanly any more and everybody who has local
changes will have to do manual merges, however doing it once in a big
step is probably less hassle then doing it bit by bit as part of
multiple commits.
What do you think, would now be a good time to do some of these
changes? I would at least apply @Overrides/@Deprecated everywhere,
remove trailing whitespaces, adjust intendation, organize imports,
remove unnecessary casts, ... and a few others, all can be done easily
via the Eclipse "clean up" support.
About Imports:
I removed the automatic adjustment of imports some time ago because
with multiple people working on the code it usually ends up in a
change-ping-pong where devs apply "their" style every time they change
the file and thus the imports are changed to one style and then to
another. CTRL-SHIFT-O in Eclipse allows to organize the imports on
demand.
Usually I try to not have to think about the imports much. Fortunately
Eclipse has support in the CTRL-1 shortcut to let one choose the class
to import without having to go up. With some special settings in
Eclipse it should also work for the junit-assertions, e.g. try CTRL-1
on an assertTrue that is not imported yet will suggest to import
junit.Assert.*.
Dominik.
On Fri, May 29, 2015 at 11:33 PM, Andreas Beeker <ki...@apache.org> wrote:
> Thank you, David and Tim.
>
> Before I ask you to go a bit more into detail, I wanted to add a few of my motivation points -
> everything is minor and I can live without it, but when choosing between wildcards and full import,
> I tend towards wildcards:
> - when writing junit4 test, every time I use a new assertXYZ, I have to switch to the imports
> add a * to the static Assert import and then go back to the code.
> - If we receive patches, the imports don't always match - using wildcards might help with it
> (yes, very weak point)
> - We used to have organize imports on save in the Eclipse settings, some day this vanished ...
> not sure why, I thought it's because of the keeping the order of the imports the same and
> to change less lines in the commit.
> If we decide to omit wildcards, I'd prefer to activate that setting again.
>
>
>> It's much easier to debug that way ...
> I haven't noticed a difference in debugging in Eclipse - is there one?
>
>> and to ensure that I have the exact ones
> I know there are ambiguities - e.g. I don't like it, when similar classes have the same name (e.g. Shape) -
> but this is more an issue of class design. One could also argue in contrast, if the classes can't be
> imported uniquely with wildcards, there's something dodgy in their naming ...
> and we'd find out earlier, before users will notice it, as we can't control their namespace optimization
>
>> In the ooxml classes, it makes it easier to confirm that I'm triggering the classes that we need downstream in ooxml-schemas for Tika.
> So there are cases where the ooxml-lite task is doing its job not correct?
> Would the classloader load all the wildcards? - this would be a definite no-go for wildcards.
> I could imagine, that it doesn't fetch indirect references (e.g. enum classes),
> but that would happen either way.
>
> Andi.
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
> For additional commands, e-mail: dev-help@poi.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
Re: style guidelines?
Posted by Dominik Stadler <do...@gmx.at>.
Hi,
I though about performing an automatic cleanup via Eclipse tooling at
some point, this would fix some of the inconsistencies across the
whole source and should provide a bit more consistency, albeit a full
automated reformatting is probably too much as it would make some code
less readable.
A big downside of this is that patches done with previous version
likely will not apply cleanly any more and everybody who has local
changes will have to do manual merges, however doing it once in a big
step is probably less hassle then doing it bit by bit as part of
multiple commits.
What do you think, would now be a good time to do some of these
changes? I would at least apply @Overrides/@Deprecated everywhere,
remove trailing whitespaces, adjust intendation, organize imports,
remove unnecessary casts, ... and a few others, all can be done easily
via the Eclipse "clean up" support.
About Imports:
I removed the automatic adjustment of imports some time ago because
with multiple people working on the code it usually ends up in a
change-ping-pong where devs apply "their" style every time they change
the file and thus the imports are changed to one style and then to
another. CTRL-SHIFT-O in Eclipse allows to organize the imports on
demand.
Usually I try to not have to think about the imports much. Fortunately
Eclipse has support in the CTRL-1 shortcut to let one choose the class
to import without having to go up. With some special settings in
Eclipse it should also work for the junit-assertions, e.g. try CTRL-1
on an assertTrue that is not imported yet will suggest to import
junit.Assert.*.
Dominik.
On Fri, May 29, 2015 at 11:33 PM, Andreas Beeker <ki...@apache.org> wrote:
> Thank you, David and Tim.
>
> Before I ask you to go a bit more into detail, I wanted to add a few of my motivation points -
> everything is minor and I can live without it, but when choosing between wildcards and full import,
> I tend towards wildcards:
> - when writing junit4 test, every time I use a new assertXYZ, I have to switch to the imports
> add a * to the static Assert import and then go back to the code.
> - If we receive patches, the imports don't always match - using wildcards might help with it
> (yes, very weak point)
> - We used to have organize imports on save in the Eclipse settings, some day this vanished ...
> not sure why, I thought it's because of the keeping the order of the imports the same and
> to change less lines in the commit.
> If we decide to omit wildcards, I'd prefer to activate that setting again.
>
>
>> It's much easier to debug that way ...
> I haven't noticed a difference in debugging in Eclipse - is there one?
>
>> and to ensure that I have the exact ones
> I know there are ambiguities - e.g. I don't like it, when similar classes have the same name (e.g. Shape) -
> but this is more an issue of class design. One could also argue in contrast, if the classes can't be
> imported uniquely with wildcards, there's something dodgy in their naming ...
> and we'd find out earlier, before users will notice it, as we can't control their namespace optimization
>
>> In the ooxml classes, it makes it easier to confirm that I'm triggering the classes that we need downstream in ooxml-schemas for Tika.
> So there are cases where the ooxml-lite task is doing its job not correct?
> Would the classloader load all the wildcards? - this would be a definite no-go for wildcards.
> I could imagine, that it doesn't fetch indirect references (e.g. enum classes),
> but that would happen either way.
>
> Andi.
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
> For additional commands, e-mail: dev-help@poi.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
Re: style guidelines?
Posted by Andreas Beeker <ki...@apache.org>.
Thank you, David and Tim.
Before I ask you to go a bit more into detail, I wanted to add a few of my motivation points -
everything is minor and I can live without it, but when choosing between wildcards and full import,
I tend towards wildcards:
- when writing junit4 test, every time I use a new assertXYZ, I have to switch to the imports
add a * to the static Assert import and then go back to the code.
- If we receive patches, the imports don't always match - using wildcards might help with it
(yes, very weak point)
- We used to have organize imports on save in the Eclipse settings, some day this vanished ...
not sure why, I thought it's because of the keeping the order of the imports the same and
to change less lines in the commit.
If we decide to omit wildcards, I'd prefer to activate that setting again.
> It's much easier to debug that way ...
I haven't noticed a difference in debugging in Eclipse - is there one?
> and to ensure that I have the exact ones
I know there are ambiguities - e.g. I don't like it, when similar classes have the same name (e.g. Shape) -
but this is more an issue of class design. One could also argue in contrast, if the classes can't be
imported uniquely with wildcards, there's something dodgy in their naming ...
and we'd find out earlier, before users will notice it, as we can't control their namespace optimization
> In the ooxml classes, it makes it easier to confirm that I'm triggering the classes that we need downstream in ooxml-schemas for Tika.
So there are cases where the ooxml-lite task is doing its job not correct?
Would the classloader load all the wildcards? - this would be a definite no-go for wildcards.
I could imagine, that it doesn't fetch indirect references (e.g. enum classes),
but that would happen either way.
Andi.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
Re: style guidelines?
Posted by David kerber <dc...@verizon.net>.
On 5/29/2015 10:31 AM, Allison, Timothy B. wrote:
> Thank you, Nick and Andi.
>
>>> Can we limit this to X classes per package or X static imports?
>
> I have a preference to avoid wildcards altogether. In the ooxml classes, it makes it easier to confirm that I'm triggering the classes that we need downstream in ooxml-schemas for Tika.
>
> I defer, though, to you and the other devs on this item.
I'm not a dev for POI, but in the code I maintain, I avoid wildcard
imports like the plague, no matter how many it needs! Cold folding in
Eclipse takes care of the visual issue.
It's much easier to debug that way, and to ensure that I have the exact
ones I want when they are all explicit.
>
> -----Original Message-----
> From: Andreas Beeker [mailto:kiwiwings@apache.org]
> Sent: Thursday, May 28, 2015 5:11 PM
> To: POI Developers List
> Subject: Re: style guidelines?
>
> On 28.05.2015 21:36, Allison, Timothy B. wrote:
>> 4) don't use wildcards on imports
>
> This is something I'd like to change.
> Can we limit this to X classes per package or X static imports?
>
> For common_sl, I've used a setting of 3 full imports, and for a higher number wildcards will be used.
> The only ambiguity which comes up regularly is java.util/awt.List
>
> It's ok if we decide differently, there's a shortcut for it in Eclipse ... before I merge the branch.
>
> Andi
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
> For additional commands, e-mail: dev-help@poi.apache.org
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
> For additional commands, e-mail: dev-help@poi.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
RE: style guidelines?
Posted by "Allison, Timothy B." <ta...@mitre.org>.
Thank you, Nick and Andi.
>> Can we limit this to X classes per package or X static imports?
I have a preference to avoid wildcards altogether. In the ooxml classes, it makes it easier to confirm that I'm triggering the classes that we need downstream in ooxml-schemas for Tika.
I defer, though, to you and the other devs on this item.
-----Original Message-----
From: Andreas Beeker [mailto:kiwiwings@apache.org]
Sent: Thursday, May 28, 2015 5:11 PM
To: POI Developers List
Subject: Re: style guidelines?
On 28.05.2015 21:36, Allison, Timothy B. wrote:
> 4) don't use wildcards on imports
This is something I'd like to change.
Can we limit this to X classes per package or X static imports?
For common_sl, I've used a setting of 3 full imports, and for a higher number wildcards will be used.
The only ambiguity which comes up regularly is java.util/awt.List
It's ok if we decide differently, there's a shortcut for it in Eclipse ... before I merge the branch.
Andi
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
Re: style guidelines?
Posted by Andreas Beeker <ki...@apache.org>.
On 28.05.2015 21:36, Allison, Timothy B. wrote:
> 4) don't use wildcards on imports
This is something I'd like to change.
Can we limit this to X classes per package or X static imports?
For common_sl, I've used a setting of 3 full imports, and for a higher number wildcards will be used.
The only ambiguity which comes up regularly is java.util/awt.List
It's ok if we decide differently, there's a shortcut for it in Eclipse ... before I merge the branch.
Andi
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org