-
Notifications
You must be signed in to change notification settings - Fork 208
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update to HAML 6 #1426
Update to HAML 6 #1426
Conversation
Code changes look like they simplify for the reader. |
de3ceee
to
0edca80
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@deivid-rodriguez Thanks for following my progress.
While I am not able check all pages today, there is a small regression at the top page as below:
- Source
Line 10 in cf57e55
Starting work on a project is as simple as <code>bundle install</code>.
0edca80
to
95c8476
Compare
Fixed, thank you! |
Analogous to the blog header one.
There were two issues: * Pages using the two column layout were duplicating content. I identified the issue as related to the weird usage of `content_for` inside `wrap_layout`. I removed that because it was also making the template structure harder to follow. * Sometimes HTML entities were not being properly interpreted. Fixed by switching to use `!=` in those cases. Signed-off-by: Takuya Noguchi <[email protected]>
95c8476
to
e542c19
Compare
I'll merge now, we can address anything else as follow ups! |
@deivid-rodriguez Thanks for finishing up! |
@tnir Thanks for getting this started! |
What was the end-user problem that led to this PR?
The problem was none. I just wanted to finish the upgrade of all our stack to latest versions.
What is your fix for the problem, implemented in this PR?
My fix is to upgrade HAML and take care of a couple of problems that showed up.
Supersedes #918.