Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

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

Created:
Dec. 5, 2017, 11:19 a.m. by ire
Modified:
Dec. 13, 2017, 7:16 p.m.
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
Dec. 5, 2017, 11:19 a.m. (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?
Dec. 5, 2017, 4:30 p.m. (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 ...
Dec. 5, 2017, 4:45 p.m. (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") }} | {{ ...
Dec. 6, 2017, 5:07 p.m. (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") }} | {{ ...
Dec. 7, 2017, 1:35 p.m. (2017-12-07 13:35:10 UTC) #5
juliandoucette
This will look good to me after you remove the translate filter pointed out below. ...
Dec. 8, 2017, 3:44 p.m. (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 ...
Dec. 11, 2017, 3:34 p.m. (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 ...
Dec. 12, 2017, 4:38 p.m. (2017-12-12 16:38:32 UTC) #8
juliandoucette
Actually, wait... Not LGTM
Dec. 12, 2017, 4:41 p.m. (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 ...
Dec. 12, 2017, 4:52 p.m. (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 ...
Dec. 12, 2017, 6:37 p.m. (2017-12-12 18:37:13 UTC) #11
ire
New patch set
Dec. 13, 2017, 9:39 a.m. (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?
Dec. 13, 2017, 3:01 p.m. (2017-12-13 15:01:10 UTC) #13
ire
Dec. 13, 2017, 7:16 p.m. (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.

Powered by Google App Engine
This is Rietveld