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

Issue 29755927: Noissue - Have sitemap sort output by page (Closed)

Created:
April 19, 2018, 3:37 a.m. by rosie
Modified:
May 1, 2018, 6:46 p.m.
Base URL:
https://hg.adblockplus.org/cms/
Visibility:
Public.

Description

Noissue - Have sitemap sort output by page

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+5 lines, -5 lines) Patch
M tests/expected_output/en/sitemap View 1 chunk +2 lines, -2 lines 0 comments Download
M tests/test_site/pages/sitemap.tmpl View 1 chunk +3 lines, -3 lines 0 comments Download

Messages

Total messages: 6
rosie
April 19, 2018, 3:37 a.m. (2018-04-19 03:37:12 UTC) #1
Sebastian Noack
LGTM
April 19, 2018, 7:06 a.m. (2018-04-19 07:06:42 UTC) #2
Vasily Kuznetsov
LGTM. I wonder if maybe `get_pages_metadata` should just sort everything this way to always produce ...
April 19, 2018, 3:32 p.m. (2018-04-19 15:32:19 UTC) #3
Sebastian Noack
On 2018/04/19 15:32:19, Vasily Kuznetsov wrote: > I wonder if maybe `get_pages_metadata` should just sort ...
April 19, 2018, 9:41 p.m. (2018-04-19 21:41:10 UTC) #4
Vasily Kuznetsov
On 2018/04/19 21:41:10, Sebastian Noack wrote: > On 2018/04/19 15:32:19, Vasily Kuznetsov wrote: > > ...
April 20, 2018, 10:20 a.m. (2018-04-20 10:20:22 UTC) #5
Jon Sonesen
April 21, 2018, 12:23 a.m. (2018-04-21 00:23:40 UTC) #6
On 2018/04/20 10:20:22, Vasily Kuznetsov wrote:
> On 2018/04/19 21:41:10, Sebastian Noack wrote:
> > On 2018/04/19 15:32:19, Vasily Kuznetsov wrote:
> > > I wonder if maybe `get_pages_metadata` should just sort everything this
way
> to
> > > always produce deterministic order. What do you guys think?
> > 
> > I think its not worth it. In a real-world scenario it would make more sense
to
> > sort by the visible title (or some other metadata), not by an internal
> > identifier like here.
> 
> Yeah, makes sense. Allright, let's not sort it.

LGTM

Powered by Google App Engine
This is Rietveld