[openstreetmap-website] Refactor view and css code around homepage links. This adds a new (#83)

Tom Hughes notifications at github.com
Thu Sep 6 09:33:35 BST 2012


I haven't assessed this in detail yet, and the actual results may be fine, but stylistically I hate many of the changes to the way the ERB is laid out.

One of the big problems with assessing this, and many of your patches, is a tendency to make functional changes to the way something is done - moving logic from ruby to JS say - and changes to the layout of code all in the one patch.

That both makes it very difficult to see where the functional changes are, because much of the diff is showing differences that are in essence just layout changes, and also makes it hard to object to stuff because it involves throwing the baby out with the bathwater.

As far as my actual concerns with style in the ERB go, things I have noticed are:

* Lots of blank lines removed - blanks lines are there to break up what would otherwise be a monolithic block of HTML into sections to make it easy to read and reason about.
* Lots of `<%= %>` templates broken up over multiple lines. Whilst I understand the wish to get rid of the very long lines I have always tried to avoid multiline templates because I've never found a way of laying them out that I like. The real problem is probably having that much ruby code in the ERB in the first place mind...
* Lots of changes to indentation, mostly reducing indentation on follow on lines so that arguments no longer line up with the initial arguments in the first line for example.

I don't necessarily know what the answer to all these layout issues is mind, which is why I was trying to start a discussion about it last week.

---
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/83#issuecomment-8325908
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20120906/2348432d/attachment.html>


More information about the rails-dev mailing list