On 15 May 2013 08:49, Thomas Thrainer <[email protected]> wrote:
>
>
>
> On Tue, May 14, 2013 at 8:15 PM, Bernardo Dal Seno <[email protected]>
> wrote:
>>
>> On 14 May 2013 15:52, Thomas Thrainer <[email protected]> wrote:
>> > LUNetwork* and associated helper functions are extracted to network.py.
>> >
>> > Signed-off-by: Thomas Thrainer <[email protected]>
>> > ---
>> >  Makefile.am            |   3 +-
>> >  lib/cmdlib/__init__.py | 714
>> > +-----------------------------------------------
>> >  lib/cmdlib/common.py   |  25 ++
>> >  lib/cmdlib/network.py  | 718
>> > +++++++++++++++++++++++++++++++++++++++++++++++++
>> >  4 files changed, 749 insertions(+), 711 deletions(-)
>> >  create mode 100644 lib/cmdlib/network.py
>>
>> In most of Ganeti code, helper functions precede their use in a file.
>> In network.py you are putting helper functions at the end of the file.
>> I think it would be better if we would be consistent on this regard.
>> Rest LGTM, thanks.
>>
> In "old" cmdlib.py, helper functions were scattered all over the place,
> sometimes defined above and sometimes below the classes which use them.
> I've put all helper functions for a module at the end of the file in this
> patch series, but if there is a guideline how to arrange them (or if we can
> agree on one), I can rearrange them.

Well, cmdlib.py is not the best example of style. What happened there
is that some functions were initially used by some LUs, then got used
by more LUs. Code was refactored, and move between LU and functions,
so I'm not surprised that the order is almost random.

Anyway, cmdlib.py as we knew it's gone (thanks, Thomas), so I'd look
at the rest of the code. I remember to have seen the same pattern many
times, with helper functions preceding their use (not necessarily at
the beginning of the file). I could be mistaken and I don't care much
about which way we order, just I'd like to have consistency. We don't
have any rule in our guide lines, but maybe we should decide for one
and add it.

Bernardo

Reply via email to