Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(361)

Issue 29630648: Issue 6013 - Add site name to document title in website-defaults standard metadata (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 week, 2 days ago by ire
Modified:
21 hours, 40 minutes ago
Reviewers:
juliandoucette
Base URL:
https://hg.adblockplus.org/website-defaults
Visibility:
Public.

Description

Issue 6013 - Add site name to document title in website-defaults standard metadata

Patch Set 1 #

Total comments: 2

Patch Set 2 : Translate page title suffix #

Total comments: 4

Patch Set 3 : Remove translation on get_string #

Total comments: 4

Patch Set 4 : Remove repeated markup #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+10 lines, -6 lines) Patch
M includes/meta/standard.tmpl View 1 2 3 1 chunk +9 lines, -5 lines 2 comments Download
M pages/index.html View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 14
ire
1 week, 2 days ago (2017-12-05 11:19:20 UTC) #1
juliandoucette
Why are you explicitly setting title_suffix to none and not *just* not defining title_suffix?
1 week, 2 days ago (2017-12-05 16:30:04 UTC) #2
ire
On 2017/12/05 16:30:04, juliandoucette wrote: > Why are you explicitly setting title_suffix to none and ...
1 week, 2 days ago (2017-12-05 16:45:58 UTC) #3
juliandoucette
https://codereview.adblockplus.org/29630648/diff/29630649/includes/meta/standard.tmpl File includes/meta/standard.tmpl (right): https://codereview.adblockplus.org/29630648/diff/29630649/includes/meta/standard.tmpl#newcode11 includes/meta/standard.tmpl:11: {{ title | translate("page-title", "Page title") }} | {{ ...
1 week ago (2017-12-06 17:07:18 UTC) #4
ire
https://codereview.adblockplus.org/29630648/diff/29630649/includes/meta/standard.tmpl File includes/meta/standard.tmpl (right): https://codereview.adblockplus.org/29630648/diff/29630649/includes/meta/standard.tmpl#newcode11 includes/meta/standard.tmpl:11: {{ title | translate("page-title", "Page title") }} | {{ ...
1 week ago (2017-12-07 13:35:10 UTC) #5
juliandoucette
This will look good to me after you remove the translate filter pointed out below. ...
6 days, 1 hour ago (2017-12-08 15:44:56 UTC) #6
ire
https://codereview.adblockplus.org/29630648/diff/29632573/includes/meta/standard.tmpl File includes/meta/standard.tmpl (right): https://codereview.adblockplus.org/29630648/diff/29632573/includes/meta/standard.tmpl#newcode4 includes/meta/standard.tmpl:4: On 2017/12/08 15:44:56, juliandoucette wrote: > Detail: This logic ...
3 days, 1 hour ago (2017-12-11 15:34:17 UTC) #7
juliandoucette
LGTM after you address my comment below. https://codereview.adblockplus.org/29630648/diff/29635574/includes/meta/standard.tmpl File includes/meta/standard.tmpl (right): https://codereview.adblockplus.org/29630648/diff/29635574/includes/meta/standard.tmpl#newcode6 includes/meta/standard.tmpl:6: <title> There ...
2 days ago (2017-12-12 16:38:32 UTC) #8
juliandoucette
Actually, wait... Not LGTM
2 days ago (2017-12-12 16:41:50 UTC) #9
juliandoucette
https://codereview.adblockplus.org/29630648/diff/29635574/includes/meta/standard.tmpl File includes/meta/standard.tmpl (right): https://codereview.adblockplus.org/29630648/diff/29635574/includes/meta/standard.tmpl#newcode5 includes/meta/standard.tmpl:5: {% if title_suffix == 'none' or (not title_suffix and ...
2 days ago (2017-12-12 16:52:45 UTC) #10
ire
https://codereview.adblockplus.org/29630648/diff/29635574/includes/meta/standard.tmpl File includes/meta/standard.tmpl (right): https://codereview.adblockplus.org/29630648/diff/29635574/includes/meta/standard.tmpl#newcode5 includes/meta/standard.tmpl:5: {% if title_suffix == 'none' or (not title_suffix and ...
1 day, 22 hours ago (2017-12-12 18:37:13 UTC) #11
ire
New patch set
1 day, 7 hours ago (2017-12-13 09:39:42 UTC) #12
juliandoucette
LGTM + NIT https://codereview.adblockplus.org/29630648/diff/29637555/includes/meta/standard.tmpl File includes/meta/standard.tmpl (right): https://codereview.adblockplus.org/29630648/diff/29637555/includes/meta/standard.tmpl#newcode7 includes/meta/standard.tmpl:7: NIT: Unnecessary empty line?
1 day, 1 hour ago (2017-12-13 15:01:10 UTC) #13
ire
21 hours, 40 minutes ago (2017-12-13 19:16:01 UTC) #14
https://codereview.adblockplus.org/29630648/diff/29637555/includes/meta/stand...
File includes/meta/standard.tmpl (right):

https://codereview.adblockplus.org/29630648/diff/29637555/includes/meta/stand...
includes/meta/standard.tmpl:7: 
On 2017/12/13 15:01:10, juliandoucette wrote:
> NIT: Unnecessary empty line? 

Done.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 87257f5