Hi Alejandro, thank you for sharing your thoughts. Yeah, touching existing code is always scary.
In my humble opinion, removing tailing spaces and converting a tab character to 4 spaces are safer. And we can probably make a task dedicated to this simple conversion, or tasks to each mvn module (e.g. one for core and another for client, etc.) and it must not include any logic change so that we can isolate in case we introduce a bug Adding a new line for lines more than 132 a bit trickier. I would like to see all code comply our code style rules but we don't have to rush for that. The most important thing is to track the current style warnings and not to increase it. On Fri, Dec 14, 2012 at 3:05 PM, Alejandro Abdelnur <[email protected]>wrote: > While I agree in theory, I would stay away from reformatting existing code > as it will make difficult to trace changes when looking for changes that > may have introduced a bug > > Thx > > > On Fri, Dec 14, 2012 at 2:32 PM, jun aoki <[email protected]> wrote: > > > Robert, I totally agree. > > I've also found that there are many code line, which probably haven't > been > > touched a while, that have tailing spaces, more than 132 lines etc. > > We should have a task ticket that dedicated to clean those things up > after > > we finalize this checkstyle thing. > > > > I can volunteer for the clean up but let's finish checkstyle first. > > > > > > > > On Fri, Dec 14, 2012 at 2:03 PM, Robert Kanter <[email protected]> > > wrote: > > > > > Hi, > > > > > > If you use a formatter in an IDE, make sure it only tries to format the > > > code relevant to your changes; if it tries to reformat the entire file, > > the > > > resulting patch will have a lot of "false" changes. > > > > > > - Robert > > > > > > > > > > > > On Fri, Dec 14, 2012 at 1:59 PM, jun aoki <[email protected]> > > wrote: > > > > > > > I hope to see one single source of truth for code style, which also > > > should > > > > be universal.(meaning not depending on a particular IDE and OSs) > > > > checkstyle is one of universal solutions so long as you use maven. > > > > > > > > There is eclipse-cs plugin to help you style your code on Eclipse. > > > > http://eclipse-cs.sourceforge.net/ > > > > There should be some equivalent to other IDEs, and at the end > > checkstyle > > > > static check for devs that code with their text editor (emcs, vim, > > etc.) > > > > > > > > > > > > > > > > > > > > On Fri, Dec 14, 2012 at 1:42 PM, Mona Chitnis <[email protected] > > > > > > wrote: > > > > > > > > > Hello, > > > > > > > > > > I have added the formatter XML as an attachment on the Developer > > > > Resources > > > > > Cwiki page > > > > > > > https://cwiki.apache.org/confluence/display/OOZIE/Developer+Resources > > > > > > > > > > P.S. Find the attachment (clip) icon on the top left. > > > > > > > > > > -- > > > > > Mona > > > > > > > > > > On 12/14/12 12:15 PM, "Saiph Kappa" <[email protected]> wrote: > > > > > > > > > > >I was also asking for the XML profile to set up in Eclipse > > > > > >(Preferences/Java/Formatter) so that every time I save a java file > > it > > > > > >formats me the code (Preferences/Java/Editor/Save Actions). > > > > > > > > > > > >I appreciate if anyone can send me the profile for oozie. > > > > > > > > > > > >Thanks. > > > > > > > > > > > >On Fri, Dec 14, 2012 at 6:03 PM, jun aoki <[email protected] > > > > > > wrote: > > > > > > > > > > > >> Alejandro/Robert, I have submitted a patch > > > > > >> https://issues.apache.org/jira/browse/OOZIE-1126 > > > > > >> Could you take a look if you get a chance? > > > > > >> > > > > > >> Oozie devs, please vote up the issue page if you are interested > in > > > > > >>seeing > > > > > >> static code check at local computer so that you would not have > to > > > wait > > > > > >> pre-commit build to see if your patch is OK. > > > > > >> > > > > > >> > > > > > >> On Thu, Dec 13, 2012 at 1:02 AM, jun aoki < > [email protected] > > > > > > > > >>wrote: > > > > > >> > > > > > >> > Hi Saiph, FYI: I have submitted a checkstyle patch. > > > > > >> > https://issues.apache.org/jira/browse/OOZIE-1126 > > > > > >> > Let me know if you have any feedback (at this point it is not > > > > > >>accepted by > > > > > >> > the commiters yet so you want to hold it until it gets > > official.) > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > On Wed, Dec 12, 2012 at 2:06 PM, Saiph Kappa < > > > [email protected] > > > > > >> >wrote: > > > > > >> > > > > > > >> >> Great :) This way, I don't need to waste time configuring > > > something > > > > > >>that > > > > > >> >> you all devs already did. > > > > > >> >> > > > > > >> >> Thanks. > > > > > >> >> > > > > > >> >> On Wed, Dec 12, 2012 at 8:39 PM, Mona Chitnis < > > > > [email protected] > > > > > > > > > > > >> >> wrote: > > > > > >> >> > > > > > >> >> > Hi Saiph, > > > > > >> >> > > > > > > >> >> > You can probably pick it off the JIRA OOZIE-1126 once the > > patch > > > > is > > > > > >> >> > uploaded and then committed. > > > > > >> >> > > > > > > >> >> > Thanks, > > > > > >> >> > > > > > > >> >> > Mona > > > > > >> >> > > > > > > >> >> > On 12/10/12 12:45 PM, "Saiph Kappa" <[email protected] > > > > > > wrote: > > > > > >> >> > > > > > > >> >> > >Can anyone send me the checkstyle configuration file to > use > > in > > > > > >> eclipse > > > > > >> >> and > > > > > >> >> > >idea intelliJ? > > > > > >> >> > > > > > > > >> >> > >On Mon, Dec 10, 2012 at 7:35 PM, Alejandro Abdelnur > > > > > >> >> > ><[email protected]>wrote: > > > > > >> >> > > > > > > > >> >> > >> gr8, thx for picking up this. > > > > > >> >> > >> > > > > > >> >> > >> > > > > > >> >> > >> On Mon, Dec 10, 2012 at 10:59 AM, jun aoki > > > > > >><[email protected] > > > > > >> > > > > > > >> >> > >>wrote: > > > > > >> >> > >> > > > > > >> >> > >> > Hi all, I have just made a ticket > > > > > >> >> > >> > https://issues.apache.org/jira/browse/OOZIE-1126 > > > > > >> >> > >> > I can probably make a patch for review. Let me know if > > any > > > > of > > > > > >>you > > > > > >> >> have > > > > > >> >> > >> > started working on it already. > > > > > >> >> > >> > > > > > > >> >> > >> > > > > > > >> >> > >> > On Mon, Dec 10, 2012 at 10:44 AM, Alejandro Abdelnur > > > > > >> >> > >><[email protected] > > > > > >> >> > >> > >wrote: > > > > > >> >> > >> > > > > > > >> >> > >> > > In a previous life we used to use checkstyle, I find > > > > > >>references > > > > > >> >> to a > > > > > >> >> > >> > maven > > > > > >> >> > >> > > plugin when doing a search but the links return 404, > > > > > >>looking at > > > > > >> >> G > > > > > >> >> > >> cache > > > > > >> >> > >> > > you can dig the docs: > > > > > >> >> > >> > > > > > > > >> >> > >> > > > > > > > >> >> > >> > > > > > > > >> >> > >> > > > > > > >> >> > >> > > > > > >> >> > >> > > > > > >> >> > > > > > > >> >> > > > > > >> > > > > > >> > > > > > > > > > > > > > > > http://webcache.googleusercontent.com/search?q=cache:uHFMs-ZjdQYJ:maven.a > > > > > >> >> > >> > > > > > >> > > > pache.org/plugins/maven-checkstyle-plugin/+&cd=1&hl=en&ct=clnk&gl=us > > > > > >> >> > >> > > > > > > > >> >> > >> > > The plugin seems avail in Apache Maven repo ,version > > > 2.9.1 > > > > > >>the > > > > > >> >> last > > > > > >> >> > >> > > release. > > > > > >> >> > >> > > > > > > > >> >> > >> > > We should try to see if it works, create a > > > checkstyle.xml > > > > > >>for > > > > > >> >> Oozie > > > > > >> >> > >> > coding > > > > > >> >> > >> > > conventions and add the check to test-patch > > > > > >> >> > >> > > > > > > > >> >> > >> > > Thx > > > > > >> >> > >> > > > > > > > >> >> > >> > > > > > > > >> >> > >> > > > > > > > >> >> > >> > > > > > > > >> >> > >> > > > > > > > >> >> > >> > > > > > > > >> >> > >> > > > > > > > >> >> > >> > > On Mon, Dec 10, 2012 at 9:29 AM, Robert Kanter > > > > > >> >> > >><[email protected]> > > > > > >> >> > >> > > wrote: > > > > > >> >> > >> > > > > > > > >> >> > >> > > > Hi, > > > > > >> >> > >> > > > > > > > > >> >> > >> > > > I'm not aware of any specific tools to > automatically > > > > > >>format > > > > > >> the > > > > > >> >> > >>code > > > > > >> >> > >> > > > specifically for Oozie, but most IDEs (I use > > NetBeans) > > > > > >>should > > > > > >> >> let > > > > > >> >> > >>you > > > > > >> >> > >> > > > auto-format the code. The "How to Contribute" > page > > > > > >> >> > >> > > > > > > > > >>here<https://cwiki.apache.org/OOZIE/how-to-contribute.html> > > > > > >> >> lists > > > > > >> >> > >> > > > the coding conventions near the bottom of the > page. > > > The > > > > > >>most > > > > > >> >> > >>common > > > > > >> >> > >> > > issue > > > > > >> >> > >> > > > I see is having lines that are too long (longer > than > > > 132 > > > > > >> >> > >>characters). > > > > > >> >> > >> > > > > > > > > >> >> > >> > > > - Robert > > > > > >> >> > >> > > > > > > > > >> >> > >> > > > > > > > > >> >> > >> > > > > > > > > >> >> > >> > > > On Sun, Dec 9, 2012 at 10:12 PM, jinwei zhu > > > > > >> >> > >><[email protected]> > > > > > >> >> > >> > > wrote: > > > > > >> >> > >> > > > > > > > > >> >> > >> > > > > Dear community, > > > > > >> >> > >> > > > > I want to contribute some codes back, but > > it > > > > > >>seems > > > > > >> >> that I > > > > > >> >> > >> need > > > > > >> >> > >> > > > > to make the code style general, so if there is > > some > > > > > >> automatic > > > > > >> >> > >>tools > > > > > >> >> > >> > to > > > > > >> >> > >> > > > > format the code ? > > > > > >> >> > >> > > > > > > > > > >> >> > >> > > > > > > > > > >> >> > >> > > > > > > > > > >> >> > >> > > > > Best Regards! > > > > > >> >> > >> > > > > > > > > > >> >> > >> > > > > > > > > >> >> > >> > > > > > > > >> >> > >> > > > > > > > >> >> > >> > > > > > > > >> >> > >> > > -- > > > > > >> >> > >> > > Alejandro > > > > > >> >> > >> > > > > > > > >> >> > >> > > > > > > >> >> > >> > > > > > >> >> > >> > > > > > >> >> > >> > > > > > >> >> > >> -- > > > > > >> >> > >> Alejandro > > > > > >> >> > >> > > > > > >> >> > > > > > > >> >> > > > > > > >> >> > > > > > >> > > > > > > >> > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > -- > Alejandro >
