> On Nov. 29, 2014, 9:57 p.m., Jarek Cecho wrote:
> > connector/connector-sdk/src/test/java/org/apache/sqoop/connector/idf/TestCSVIntermediateDataFormat.java,
> >  lines 21-52
> > <https://reviews.apache.org/r/28537/diff/1/?file=778388#file778388line21>
> >
> >     Can we please not rearange imports as part of this patch?
> 
> Veena Basavaraj wrote:
>     Let me highlight one thing.
>     
>     there needs to be a standard we follow in all files.
>     
>     
>     
>     In the CSVIntermediateDataFormat. the order is:
>     
>     com
>     org
>     java
>     javax
>     I put this in the "IDE" fomratter.
>     
>     the test file has some other random order. we cannot have this, it is 
> super in productive, if you want this to continue, lets stop been anal about 
> such a trivial thing.
>     
>     My 2 cents, it would be good for the project to have a standardized order 
> when I import new things, I use a shortcut in IDE and let the formatter take 
> care of it.
>     
>     I dont manually type in every new import, that is not my good use of time.
> 
> Jarek Cecho wrote:
>     You're correct that we are missing proper guidelines on how the import 
> should be ordered and I'm all for to define that. We should however have a 
> discussion about that in separate JIRA, not part of a different unrelated 
> JIRA.
>     
>     I'm wondering how is this making you a less productive? I'm not typing 
> each import manually myself, I'm using my IDE (InteliJ) to do that for me. 
> All I'm asking to not rearange the imports automatically.
> 
> Veena Basavaraj wrote:
>     If you read my comment above carefully, I have even told you to the order 
> I use in the IDE, based on the CSVIntermediateDataFormat class, and this 
> thing seems to be custom order in each file.
>     
>     What is your issue with sticking to one order that is in 
> CSVIntermediateDataFormat
>     
>     com
>     org
>     java
>     javax
>     
>     Really you think spending time on a RB on import prder is the best use of 
> your time and my time? An IDE has one set of rules. When I do CLTR+SHIFT+O, I 
> have not seen an IDE that has rules per file yet,

I don't have a problem with defining import order in the project. I however 
believe that such change should be discussed openly in standalone JIRA rather 
then on Review board as a part of unrelated change.

I'm not familiar with CTLR+SHIFT+O shortcut. Doing little bit of googling it 
seems that it's an Eclipse specific shortcut to re-organize imports. Let me 
know if I'm incorrect here. If that is right, then until we will have set of 
rules to follow, it seems to me that we can simply not call this shortcut? If 
all that the shortcut does is to reorganize the imports, then I'm assuming it 
won't have any impact on productivity?

The reason why we are trying not to have unrelated changes in patches (such as 
re-organizing imports, ...) is that doing unrelated changes makes difficult to 
use commands such as "git blame" and "git cherrypick" that we are using very 
often.


- Jarek


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28537/#review63297
-----------------------------------------------------------


On Nov. 29, 2014, 11:09 p.m., Veena Basavaraj wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28537/
> -----------------------------------------------------------
> 
> (Updated Nov. 29, 2014, 11:09 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-1750
>     https://issues.apache.org/jira/browse/SQOOP-1750
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> see jira
> 
> 
> Diffs
> -----
> 
>   
> connector/connector-sdk/src/main/java/org/apache/sqoop/connector/idf/CSVIntermediateDataFormat.java
>  4f2baf9 
>   
> connector/connector-sdk/src/test/java/org/apache/sqoop/connector/idf/TestCSVIntermediateDataFormat.java
>  b629897 
> 
> Diff: https://reviews.apache.org/r/28537/diff/
> 
> 
> Testing
> -------
> 
> yes
> 
> 
> Thanks,
> 
> Veena Basavaraj
> 
>

Reply via email to