On 21 Apr 2009, at 19:09, Adam Winer wrote:

On Tue, Apr 21, 2009 at 10:21 AM,  <[email protected]> wrote:
The import order doesnt look quite right on some of these classes, but
it didnt look right to start with ?

also,
I am slightly concerned about binding the SPI to javax.servlet. Just a
gut feeling that binding the protocol impl all the way down the stack
means you cant use another protocol... but thats probably just unfounded
as this is all about http.

In all cases, the binding is purely to the HttpServletResponse list of
HTTP error codes.  It makes me a tad uncomfortable, but just barely.
And as they're all integer constants, this is only a compile
dependency.

Agreed, also safer that they come from an 'official' source.



http://codereview.appspot.com/45044/diff/1/22
File
java/gadgets/src/main/java/org/apache/shindig/gadgets/http/ InvalidationHandler.java
(right):

http://codereview.appspot.com/45044/diff/1/22#newcode31
Line 31: import javax.servlet.http.HttpServletResponse;
I have the following import order, as configured some time last year
when the code style was agreed.

8=javax
7=java
6=org
5=org.apache.abdera
4=org.apache.shindig
3=net
2=junit
1=com
0=com.google


0 is the first


I dont think either before or after the patch is correct. (might be
wrong about that)

Oh.  I've been using a reaaally different order, from first to last:
 org
 java
 javax
 com

I don't see anything about import order in our code style webpage, and
it seems there's varying opinions in general, e.g. a message from
Kevin on another patch:

Your import order is off -- org.apache.shindig should come before the third
party code

So I'm just confused.  I'd like to punt on the import order and
revisit with a later checkin after a quick shindig-dev thread to get
everyone agreed.


Im ok with that, no further action on this commit, thanks for bringing it up on dev.
Ian


Reply via email to