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

Issue 29495555: Fixes 5343 - add global function: get_canonical_url (Closed)

Created:
July 23, 2017, 4:35 p.m. by Vasily Kuznetsov
Modified:
Aug. 4, 2017, 6:17 p.m.
CC:
ire, saroyanm
Visibility:
Public.

Description

Fixes 5343 - add global function: get_canonical_url Repository: https://hg.adblockplus.org/cms Base revision: b83af9f98bb3

Patch Set 1 #

Total comments: 3

Patch Set 2 : Implement get_canonical_url (according to updated ticket) #

Patch Set 3 : Add comment about test-type-specific expected output handling #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+57 lines, -10 lines) Patch
M cms/converters.py View 1 3 chunks +18 lines, -0 lines 0 comments Download
A tests/expected_output/get_page_url@dynamic View 1 1 chunk +5 lines, -0 lines 0 comments Download
A tests/expected_output/get_page_url@static View 1 1 chunk +5 lines, -0 lines 0 comments Download
M tests/expected_output/sitemap View 1 chunk +2 lines, -2 lines 0 comments Download
M tests/test_page_outputs.py View 1 2 2 chunks +19 lines, -8 lines 1 comment Download
A tests/test_site/pages/get_page_url.tmpl View 1 1 chunk +7 lines, -0 lines 0 comments Download
M tests/test_site/settings.ini View 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 22
Vasily Kuznetsov
Hi Jon and Julian! Here's an implementation of `get_page_url`. At the moment it has two ...
July 23, 2017, 4:55 p.m. (2017-07-23 16:55:33 UTC) #1
juliandoucette
On 2017/07/23 16:55:33, Vasily Kuznetsov wrote: > 1. It allows generating URLs of pages that ...
July 24, 2017, 3:04 p.m. (2017-07-24 15:04:13 UTC) #2
Vasily Kuznetsov
Thanks for the answers, Julian, see below. On 2017/07/24 15:04:13, juliandoucette wrote: > On 2017/07/23 ...
July 24, 2017, 4:05 p.m. (2017-07-24 16:05:38 UTC) #3
juliandoucette
CC ire, saroyanm On 2017/07/24 16:05:38, Vasily Kuznetsov wrote: > Indeed, the code that generates ...
July 24, 2017, 5:29 p.m. (2017-07-24 17:29:45 UTC) #4
juliandoucette
On 2017/07/24 17:29:45, juliandoucette wrote: > I'm concerned about this function being used by mistake ...
July 24, 2017, 5:33 p.m. (2017-07-24 17:33:45 UTC) #5
juliandoucette
On 2017/07/24 17:29:45, juliandoucette wrote: > I think that a function called `get_page_url` should throw ...
July 24, 2017, 5:41 p.m. (2017-07-24 17:41:04 UTC) #6
juliandoucette
On 2017/07/24 17:41:04, juliandoucette wrote: > On 2017/07/24 17:29:45, juliandoucette wrote: > > I think ...
July 24, 2017, 5:48 p.m. (2017-07-24 17:48:19 UTC) #7
juliandoucette
> I'm concerned about this function being used by mistake in the future to > ...
July 24, 2017, 6:05 p.m. (2017-07-24 18:05:12 UTC) #8
Vasily Kuznetsov
On 2017/07/24 18:05:12, juliandoucette wrote: > > I'm concerned about this function being used by ...
July 25, 2017, noon (2017-07-25 12:00:43 UTC) #9
juliandoucette
On 2017/07/25 12:00:43, Vasily Kuznetsov wrote: > Does this sounds about right? If so, I ...
July 25, 2017, 12:34 p.m. (2017-07-25 12:34:54 UTC) #10
Vasily Kuznetsov
On 2017/07/25 12:34:54, juliandoucette wrote: > On the other hand, I think I prefer: > ...
July 25, 2017, 8:42 p.m. (2017-07-25 20:42:08 UTC) #11
juliandoucette
On 2017/07/25 20:42:08, Vasily Kuznetsov wrote: > On 2017/07/25 12:34:54, juliandoucette wrote: > > On ...
July 26, 2017, 12:12 p.m. (2017-07-26 12:12:46 UTC) #12
juliandoucette
Minor point/correction: <a {{ page | href }}> should have been: <a {{ page | ...
July 26, 2017, 12:17 p.m. (2017-07-26 12:17:03 UTC) #13
Vasily Kuznetsov
On 2017/07/26 12:12:46, juliandoucette wrote: > On 2017/07/25 20:42:08, Vasily Kuznetsov wrote: > > While ...
July 26, 2017, 5:56 p.m. (2017-07-26 17:56:54 UTC) #14
Vasily Kuznetsov
I implemented get_canonical_url now, as described in the updated ticket.
July 28, 2017, 10:08 a.m. (2017-07-28 10:08:07 UTC) #15
juliandoucette
On 2017/07/28 10:08:07, Vasily Kuznetsov wrote: > I implemented get_canonical_url now, as described in the ...
July 28, 2017, 11:43 p.m. (2017-07-28 23:43:07 UTC) #16
Jon Sonesen
On 2017/07/28 23:43:07, juliandoucette wrote: > On 2017/07/28 10:08:07, Vasily Kuznetsov wrote: > > I ...
July 28, 2017, 11:47 p.m. (2017-07-28 23:47:27 UTC) #17
Jon Sonesen
On 2017/07/28 23:43:07, juliandoucette wrote: > On 2017/07/28 10:08:07, Vasily Kuznetsov wrote: > > I ...
July 28, 2017, 11:47 p.m. (2017-07-28 23:47:29 UTC) #18
Jon Sonesen
On 2017/07/28 23:47:29, Jon Sonesen wrote: > On 2017/07/28 23:43:07, juliandoucette wrote: > > On ...
Aug. 2, 2017, 6:20 p.m. (2017-08-02 18:20:57 UTC) #19
Vasily Kuznetsov
On 2017/08/02 18:20:57, Jon Sonesen wrote: > On 2017/07/28 23:47:29, Jon Sonesen wrote: > > ...
Aug. 2, 2017, 6:32 p.m. (2017-08-02 18:32:05 UTC) #20
Jon Sonesen
On 2017/08/02 18:32:05, Vasily Kuznetsov wrote: > On 2017/08/02 18:20:57, Jon Sonesen wrote: > > ...
Aug. 2, 2017, 6:48 p.m. (2017-08-02 18:48:55 UTC) #21
Vasily Kuznetsov
Aug. 3, 2017, 9:12 a.m. (2017-08-03 09:12:20 UTC) #22
Hi Jon,

Good point about adding a comment that explains test-type-specific expected
output handling. I added it.

Cheers,
Vasily

https://codereview.adblockplus.org/29495555/diff/29504563/tests/test_page_out...
File tests/test_page_outputs.py (right):

https://codereview.adblockplus.org/29495555/diff/29504563/tests/test_page_out...
tests/test_page_outputs.py:25: # Move test-type-specific expected outputs (e.g.
"xyz@static" -> "xyz")
I added a comment to make it more clear what's going on here.

Powered by Google App Engine
This is Rietveld