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

Issue 29359623: Issue 3819 - Add Jobs link to the footer (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 years ago by saroyanm
Modified:
3 years ago
Reviewers:
juliandoucette
Visibility:
Public.

Description

Issue 3819 - Add Jobs link to the footer

Patch Set 1 #

Total comments: 3
Unified diffs Side-by-side diffs Delta from patch set Stats (+4 lines, -0 lines) Patch
M locales/en/menu.json View 1 chunk +3 lines, -0 lines 0 comments Download
M templates/default.tmpl View 1 chunk +1 line, -0 lines 3 comments Download

Messages

Total messages: 4
saroyanm
3 years ago (2016-10-25 10:53:45 UTC) #1
juliandoucette
Quick question. https://codereview.adblockplus.org/29359623/diff/29359624/templates/default.tmpl File templates/default.tmpl (right): https://codereview.adblockplus.org/29359623/diff/29359624/templates/default.tmpl#newcode185 templates/default.tmpl:185: <li><a href="https://eyeo.com/jobs/" hreflang="en">{{get_string("jobs", "menu")}}</a></li> Why are we ...
3 years ago (2016-10-25 11:15:23 UTC) #2
saroyanm
https://codereview.adblockplus.org/29359623/diff/29359624/templates/default.tmpl File templates/default.tmpl (right): https://codereview.adblockplus.org/29359623/diff/29359624/templates/default.tmpl#newcode185 templates/default.tmpl:185: <li><a href="https://eyeo.com/jobs/" hreflang="en">{{get_string("jobs", "menu")}}</a></li> On 2016/10/25 11:15:22, juliandoucette wrote: ...
3 years ago (2016-10-25 12:09:19 UTC) #3
juliandoucette
3 years ago (2016-10-25 12:15:46 UTC) #4
LGTM

https://codereview.adblockplus.org/29359623/diff/29359624/templates/default.tmpl
File templates/default.tmpl (right):

https://codereview.adblockplus.org/29359623/diff/29359624/templates/default.t...
templates/default.tmpl:185: <li><a href="https://eyeo.com/jobs/"
hreflang="en">{{get_string("jobs", "menu")}}</a></li>
On 2016/10/25 12:09:19, saroyanm wrote:
> On 2016/10/25 11:15:22, juliandoucette wrote:
> > Why are we using get_string here?
> 
> get_string is using a file to select the string, if I'll use translate
function
> I'll need to specify the translation for each page separately, I think this is
> an issue as well in the AA website (ex.:
>
https://hg.adblockplus.org/web.acceptableads.com/file/tip/includes/navbar.tmp...)
> still needs to be addressed in the reviews I guess.
> If you want to see the result in crowdin, you can run the test repo I created
> for the translation test on test crowdin project we have:
>
https://bitbucket.org/saroyanm/translation-script-test/src/17b537493d4fdb551f...
> 

Acknowledged.
Sign in to reply to this message.

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