When running ruby 1.8.5 (latest, stable) a bunch of warnings show up. activerecord/lib/active_record/connection_adapters/oracle_adapter.rb:500: warning: colon will be obsoleted; use semicolon It seems that edge ruby wants this form for case statements: case stuff when 5; something when 6; something_else end This patch addresses this issue, as well as cleaning up a bunch of inconsistent conditional code style (mainly, stripping out 'then' in the few places it occurred)
on 2006-07-31 21:52
on 2006-08-01 03:54
Haha.. it kinda makes sense, since the semicolon is breaking a line.. I guess my reason for the patch was that there is inconsistency across rails, with a mix of then, colon, semicolon, and newline.. any thoughts on what it should be?
on 2006-08-01 04:50
> I guess my reason for the patch was that there is inconsistency across > rails, with a mix of then, colon, semicolon, and newline.. any > thoughts on what it should be? Very much depends on what makes sense in the context. I often use different conditional styles depending on the particulars. Not using the same style everywhere is not necessary a bug. -- David Heinemeier Hansson http://www.loudthinking.com -- Broadcasting Brain http://www.basecamphq.com -- Online project management http://www.backpackit.com -- Personal information manager http://www.rubyonrails.com -- Web-application framework
on 2006-08-01 06:10
> ah! but is consistency a strength?
Context beats consistency every time. But if the context is
unimportant, then consistency is preferable to randomness.
I'm saying that the context probably did matter in quite a few of the
choices. I know I've personally used all the different styles at
different times depending on the context.
--
David Heinemeier Hansson
http://www.loudthinking.com -- Broadcasting Brain
http://www.basecamphq.com -- Online project management
http://www.backpackit.com -- Personal information manager
http://www.rubyonrails.com -- Web-application framework
on 2006-08-01 06:59
> > ah! but is consistency a strength? > > Context beats consistency every time. But if the context is > unimportant, then consistency is preferable to randomness. Yeah!! > I'm saying that the context probably did matter in quite a few of the > choices. I know I've personally used all the different styles at > different times depending on the context. It felt like, from looking through the code (and svn blame), that a bunch of different people's styles had seeped into the codebase. Perhaps now that there are so many core committers, some coding standard could be set? In some cases, (mainly routing), there is a bunch of whitespace to help readability when there are a few similar statements (which is clear and disambiguating) like case foo something : do_thing(:monkeys) other : do_thing(:bananas) else return end but elsewhere, the colon was in different places (right after the conditional, or a space before the statement). I don't speak japanese, but this seems to be relevant: http://shakenbu.org/yanagi/d/20060718.html#p01 Anyone care to make a broad, sweeping "this is the preferred way" statement for when context is somewhat neutral? q1: separator on short-line conditionals (a) then (b) semicolon (c) colon (screw matz! we're taking it back!) q2: spacing (a) separator with the conditional (when blah: stuff) (b) separator lined up (when blah : stuff)
on 2006-08-03 14:23
On 8/1/06, Courtenay <court3nay@gmail.com> wrote: > > > In some cases, (mainly routing), there is a bunch of whitespace to > help readability... This is just the sign of an ongoing battle between Jamis and I. Eventually one will emerge the winner, and the other will stop using so much whitespace. ;-)
on 2006-08-03 14:23
Nicholas Seckar wrote: > This is just the sign of an ongoing battle between Jamis and I. > Eventually one will emerge the winner, and the other will stop using so > much whitespace. > Soon, the entire Rails code-base will be a bombed-out wasteland, and Nicholas and I will have to turn to other successful open-source projects in which to wage our war. ONLY ONE WILL BE VICTORIOUS. - Jamis
Enable email notification
Enable multi-page view