← Back to team overview

openerp-community-reviewer team mailing list archive

Re: [Merge] lp:~savoirfairelinux-openerp/lp-community-utils/nag_refactor into lp:lp-community-utils

 

On 03/07/2014 07:45 PM, Sandy Carter (http://www.savoirfairelinux.com) wrote:
@gbaconnier-c2c
Good point about adding functions to Nag, the reason for turning it into a class is design and optimization.
The rationale about design is very subjective and the one about optimization is dubious:

http://hastebin.com/neyedexiju.py

Anyway, if it needs methods, and I think the display should be one, then it makes sense.

It gives the option, like to said, to add functions but also, the ability to inherit from it.

I do have big plans with the lib, for example, automatic diff-validating in the style of (https://github.com/torvalds/linux/blob/master/scripts/checkpatch.pl), specifically checking style, seeing if the diff introduces pep8 errors, removes them, testing the patches with jenkins, commenting on the MPs, etc.
I have proposed something already: https://code.launchpad.net/~savoirfairelinux-openerp/lp-community-utils/branch_pep8/+merge/205260

As for the --authenticated option, I think I took it away because it wasn't referenced or used, I can put it back now, however I will have no impact. This was a while ago, though, I could have just taken it out accidentally.
That's right at the time you made the proposal, the option seemed to be not useful.

The problem with age was that it declared it negatively, then did * -1 again while sorting. I just took out the (-1 * -1 = 1) redundancy. It also has a one off error where an MP from today is displayed as 1 day old.

Ok.



Follow ups

References