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

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 years, 4 months ago by ire
Modified:
2 years, 4 months ago
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: 5

Patch Set 2 : Use config.has_option, use title_exclude_sitename #

Total comments: 2

Patch Set 3 : Rephrase if statement #

Total comments: 2

Patch Set 4 : Use get_string to get site name #

Total comments: 4
Unified diffs Side-by-side diffs Delta from patch set Stats (+13 lines, -1 line) Patch
M includes/meta/standard.tmpl View 1 2 3 1 chunk +6 lines, -0 lines 0 comments Download
A locales/en/site.json View 1 2 3 1 chunk +5 lines, -0 lines 1 comment Download
M pages/index.html View 1 1 chunk +1 line, -0 lines 0 comments Download
M settings.ini View 1 2 3 1 chunk +1 line, -1 line 3 comments Download

Messages

Total messages: 16
ire
2 years, 4 months ago (2017-11-28 09:11:14 UTC) #1
ire
https://codereview.adblockplus.org/29622568/diff/29622569/includes/meta/standard.tmpl File includes/meta/standard.tmpl (right): https://codereview.adblockplus.org/29622568/diff/29622569/includes/meta/standard.tmpl#newcode5 includes/meta/standard.tmpl:5: {% if page == "index" or config.get("general", "sitename") is ...
2 years, 4 months ago (2017-11-28 09:12:49 UTC) #2
juliandoucette
https://codereview.adblockplus.org/29622568/diff/29622569/includes/meta/standard.tmpl File includes/meta/standard.tmpl (right): https://codereview.adblockplus.org/29622568/diff/29622569/includes/meta/standard.tmpl#newcode5 includes/meta/standard.tmpl:5: {% if page == "index" or config.get("general", "sitename") is ...
2 years, 4 months ago (2017-11-28 14:49:22 UTC) #3
ire
https://codereview.adblockplus.org/29622568/diff/29622569/includes/meta/standard.tmpl File includes/meta/standard.tmpl (right): https://codereview.adblockplus.org/29622568/diff/29622569/includes/meta/standard.tmpl#newcode5 includes/meta/standard.tmpl:5: {% if page == "index" or config.get("general", "sitename") is ...
2 years, 4 months ago (2017-11-28 16:11:26 UTC) #4
ire
New patch set uploaded
2 years, 4 months ago (2017-11-28 16:28:39 UTC) #5
juliandoucette
CC Vasily && Jon In this PatchSet Ire is trying to add a website name ...
2 years, 4 months ago (2017-11-29 20:46:17 UTC) #6
Jon Sonesen
On 2017/11/29 20:46:17, juliandoucette wrote: > CC Vasily && Jon > > In this PatchSet ...
2 years, 4 months ago (2017-11-30 11:41:05 UTC) #7
Vasily Kuznetsov
On 2017/11/29 20:46:17, juliandoucette wrote: > CC Vasily && Jon > > In this PatchSet ...
2 years, 4 months ago (2017-11-30 12:51:27 UTC) #8
Vasily Kuznetsov
Hey Ire, A small nit about the template logic. Cheers, Vasily https://codereview.adblockplus.org/29622568/diff/29622610/includes/meta/standard.tmpl File includes/meta/standard.tmpl (right): ...
2 years, 4 months ago (2017-11-30 12:53:29 UTC) #9
ire
On 2017/11/30 12:53:29, Vasily Kuznetsov wrote: > Hey Ire, > > A small nit about ...
2 years, 4 months ago (2017-11-30 13:20:00 UTC) #10
juliandoucette
Thank you for your feedback Jon & Vasily. Can I rely on you to create ...
2 years, 4 months ago (2017-12-01 15:37:44 UTC) #11
Vasily Kuznetsov
On 2017/12/01 15:37:44, juliandoucette wrote: > Thank you for your feedback Jon & Vasily. Can ...
2 years, 4 months ago (2017-12-01 15:49:11 UTC) #12
ire
https://codereview.adblockplus.org/29622568/diff/29625604/includes/meta/standard.tmpl File includes/meta/standard.tmpl (right): https://codereview.adblockplus.org/29622568/diff/29625604/includes/meta/standard.tmpl#newcode8 includes/meta/standard.tmpl:8: <title>{{ title | translate("page-title", "Page title") }} | {{ ...
2 years, 4 months ago (2017-12-04 09:10:09 UTC) #13
juliandoucette
LGTM https://codereview.adblockplus.org/29622568/diff/29628710/locales/en/site.json File locales/en/site.json (right): https://codereview.adblockplus.org/29622568/diff/29628710/locales/en/site.json#newcode3 locales/en/site.json:3: "message": "eyeo Website Defaults" suggest: Add a message ...
2 years, 4 months ago (2017-12-04 10:33:09 UTC) #14
ire
https://codereview.adblockplus.org/29622568/diff/29628710/settings.ini File settings.ini (right): https://codereview.adblockplus.org/29622568/diff/29628710/settings.ini#newcode5 settings.ini:5: On 2017/12/04 10:33:08, juliandoucette wrote: > Did you mean ...
2 years, 4 months ago (2017-12-04 10:41:09 UTC) #15
juliandoucette
2 years, 4 months ago (2017-12-04 10:50:51 UTC) #16
https://codereview.adblockplus.org/29622568/diff/29628710/settings.ini
File settings.ini (right):

https://codereview.adblockplus.org/29622568/diff/29628710/settings.ini#newcode5
settings.ini:5: 
On 2017/12/04 10:41:09, ire wrote:
> On 2017/12/04 10:33:08, juliandoucette wrote:
> > Did you mean to add a sitename?
> 
> Is that still needed if we're not using it from this config?

No ( oops :D )
Sign in to reply to this message.

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