corinthia-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andrea Pescetti <pesce...@apache.org>
Subject Re: svn commit: r1649986 - /incubator/corinthia/www/index.html
Date Wed, 07 Jan 2015 12:15:04 GMT
On 07/01/2015 jan i wrote:
> Having now studied your patch, I have 3 questions:

I suggest to set Reply-To for the commits list to dev and assume that 
people who subscribe to commits read dev and that all mails to commits 
are automatically originated.

> - Is it on purpose you changed all lines in the file....that is kind of
> very brutal.

This is just due to broken line ending detection. 
http://svn.apache.org/viewvc/incubator/corinthia/www/index.html?r1=1649890&r2=1649986&pathrev=1649986&diff_format=h

shows it correctly, it's one line.

> - Why only change one file, this effect is on all pages.

This is correct, but maybe the problem is with repeating the same footer 
across all pages by copy-paste. Anyway yes, it's surely better to 
enforce consistency of the footer if we have no technology to include it 
automatically (but at the very least I expect that SSI is enabled).

> - What is the urgency, that needed a "brute force fix" instead of doing it
> correct

Actually this is kind of the right way to do it. I mean:
- Changing the Foundation files is a no-go. You want to be able to 
update to a newer version of the framework. So what Dennis thought to be 
the "right way" is not the right way at all: Foundation files are to be 
considered read-only.
- Overriding default settings with a custom CSS file loaded after the 
default ones is what you should do and what I recommend as a good 
practice, even though I never played with Foundation.
- Missing that, inline styles are an acceptable trade-off.

> It breaks the responsive design.

No it doesn't, since it does not make changes that are relevant for the 
box model. This is true of this particular case only, of course.

By the way, I started fixing it yesterday but then saw Dorte's mail and 
stopped. Anyway, if you modify line 83 at the link above from
<div class="row">
to
<div class="row columns">
you solve the margin problem Dennis was unhappy with. But I didn't have 
time to check whether this is semantically the best solution.

Regards,
   Andrea.

Mime
View raw message