Hello John,

I started to have a bit of a look at this patch but ended up spending 
the time trying to figure out how to make roundup display in another 
language (as a native English speaker I've not had to deal with i18n 
stuff much). By the time I got the i18n stuff working I ran out of time 
and haven't had a chance to get back to looking at it.

One thing that may help in reviewing that patch is if Anthony could 
outline some steps to show what this patch does. ie. a set of steps to 
show what the current behaviour is, and another set of steps showing the 
difference in behaviour after the patch is applied. Then I may have a 
bit more of an idea about what needs to be checked.

The changes hit a lot of different files, which raises concerns about if 
this is the best approach (I'm not saying it is a wrong approach, just 
that it needs looking at with an eye to future maintainability).

At a quick glance there are also a number of unrelated changes (ie. 
random white-space fixes) in the patch which also raises flags that the 
patch may include unrelated changes and the patch will need to be 
cleaned up before I could be applied[1].

SeeYa,
John.

[1] I am of the opinion that white-space changes should only be made 
when code bordering that white-space is being modified. Otherwise it has 
a tendency to introduce bugs that are totally unrelated to intended 
changes being made and could have been avoided.

On 28/06/15 14:05, John Rouillard wrote:
>
> John Rouillard added the comment:
>
> John/jerrykahn bump. Have you had a chance to look at this?
>
> issue2550888
>
> may need some feedback on this.
>
> ----------
> nosy: +rouilj
>
> ________________________________________________
> Roundup tracker <issues@roundup-tracker.org>
> <http://issues.roundup-tracker.org/issue2550871>
> ________________________________________________
>