Hello Allen, I looked at last patch and personally I don't see any major issue, perhaps JVM flags, URLs and configuration keys might be better to define as environment variables if you want to reduce the length of the lines but I think legibility is more important.
cheers, esteban. -- Cloudera, Inc. On Sat, Jul 26, 2014 at 7:29 PM, Chris Embree <cemb...@gmail.com> wrote: > Allen, et. al., > > I'm not sure how up to date that checklist is, but imposing such a small > size on cmd arguments seems incredibly short sighted. I'm pretty sure that > it is not a generally accepted limit. I've seen MANY Hadoop processes > require lengthy CLASS_PATHS that were easily over 240 chars. > > BTW, the BASH limit is something around 24k, well beyond 120 chars. > > IMHO, if you want to avoid Bill Gates syndrome, (no one will ever need more > than 256k RAM) you might want to set an upper limit around 64K. And then > I'd use a configuration file so I could be stupid and request a max of > 256M, because I'm doing something way abnormal. > > $0.02. YMMV. ;) > > > On Sat, Jul 26, 2014 at 10:02 PM, Paresh Yadav <pya...@hortonworks.com> > wrote: > > > Hey Allen, > > > > I am not a shell scripting expert but I have written few and used/seen > many > > from including top 3 enterprise software giants. I don't think everyone > > sticks to 80 char guidelines, may be this is remnant of the old 80 char > > terminals. I prefer long descriptive names for the env vars (or vars in > > general) as it makes the program more readable. Not sure what are > technical > > ramifications of having lines longer than 80 char if any. > > > > *Thanks,* > > Paresh Yadav > > Solutions Engineer, Canada > > > > Phone: 416.688.1003 > > Email: pya...@hortonworks.com > > Website: http://www.hortonworks.com/ > > > > *Follow Us: * > > < > > > http://facebook.com/hortonworks/?utm_source=WiseStamp&utm_medium=email&utm_term=&utm_content=&utm_campaign=signature > > > > > > > < > > > http://twitter.com/hortonworks?utm_source=WiseStamp&utm_medium=email&utm_term=&utm_content=&utm_campaign=signature > > > > > < > > > http://www.linkedin.com/company/hortonworks?utm_source=WiseStamp&utm_medium=email&utm_term=&utm_content=&utm_campaign=signature > > > > > > > [image: photo] > > > > > > On Sat, Jul 26, 2014 at 3:20 PM, Allen Wittenauer <a...@apache.org> wrote: > > > > > Hey folks: > > > > > > Deep linked by http://wiki.apache.org/hadoop/CodeReviewChecklist is > > the > > > rule that line length should be ideally maximum 80 chars. (Sun coding > > > guidelines.) In general, it's a good idea and it works for many many > > > languages... > > > > > > Now the caveat. > > > > > > As most of you know, I've been hacking on HADOOP-9902 off and on > for a > > > year now. [For those that don't, this is an almost complete rewrite > of > > > most of the major shell code that we ship with Hadoop. The stuff that > > was > > > missed I'll pick it up after this gets committed.] As part of this, I > > > recently reformatted the last patch to fit that 80 character > requirement > > as > > > best I could. The result is... not good. Not good at all. In many > > ways, > > > it actually hurt readability even beyond the lack of indentation that > > Bash > > > Beautifier doesn't support for line continuation. (That case statement > in > > > bin/hadoop is painful to look at and makes me cry.) > > > > > > Barring anymore feedback, it's pretty much ready to commit. But > before > > > that happens, do we want to specify that bash has different line length > > > requirements? Say 120 chars? Most of the problems stem from our usage > > of > > > REALLY LONG env var names that can't really be changed at this point > > > without *massively* screwing backward compatibility. (Hello, > > > YARN_RESOURCEMANAGER_OPTS... I'm talking about you!). > > > > > > Bouncing the idea around a few folks, they all seem to agree that 80 > is > > > just too hard for bash given our general use case, but I think it'd be > > good > > > to have something official. > > > > > > Thoughts? > > > > > > Thanks. > > > > > > > -- > > CONFIDENTIALITY NOTICE > > NOTICE: This message is intended for the use of the individual or entity > to > > which it is addressed and may contain information that is confidential, > > privileged and exempt from disclosure under applicable law. If the reader > > of this message is not the intended recipient, you are hereby notified > that > > any printing, copying, dissemination, distribution, disclosure or > > forwarding of this communication is strictly prohibited. If you have > > received this communication in error, please contact the sender > immediately > > and delete it from your system. Thank You. > > >
