> On Nov. 29, 2014, 1: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, > > Jarek Cecho wrote: > 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.
not calling the shortcut means I manually type in every import. It adds the imports that are missing as well. kafka project for instance does not really worry about the import order. I have spent time to create a wiki on style guide and sent the email to dev@. High time we had this. I vote for google style guide that is standard across many projects. If you have others in mind, leave a feedback there. If we enforce a standard and there is a import re-ordering this means that it is part of the change and doing a git blame should be self-explanatory. - Veena ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28537/#review63297 ----------------------------------------------------------- On Nov. 29, 2014, 3: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, 3: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 > >
