Re: PATCHES: more refactoring and skeletal cvs2hg

Greg Ward wrote:
> On Sat, Aug 29, 2009 at 4:59 PM, Michael Haggerty<[email protected]> wrote:
>>> 2) GitRunOptions, SVNRunOptions: use super() where appropriate.
>>> http://vc.gerg.ca/hg/cvs2svn-patches/file/tip/use-super.patch
>> What exactly is the advantage of using super() in a trivial inheritance
>> tree (i.e., in the absence of multiple inheritance)?  It seems to me
>> that it just adds verbosity without making the code less error-prone.
>> No doubt I have overlooked something...
> 
> IMHO it *does* make the code less error-prone: you can modify the
> class hierarchy without touching all the super() calls.  E.g. with
> old-style superclass calls, changing from
> 
>   A -> B       ("A is superclass of B")
> 
> to
> 
>   A -> A1 -> B
> 
> means that you also have to update all the A.xxx(self, ...) calls in
> B.  With super(), no such change is necessary.  I've always assumed
> that was the main benefit of super().  Unfortunately, it still doesn't
> protect you from renaming B or moving methods between classes.

Yes, exactly.  I think renaming classes and changing class hierarchies
happen with comparable frequency, so it seem that you are gaining in one
situation and losing in the other at a high cost in verbosity.  It's
just a pity that Python doesn't have a nicer way of spelling super(...).

> (To paraphrase Larry Wall, "Java has to be better at something". ;-)

:-)

> Regardless: let me know if you want me to drop the patch.  No feelings
> hurt either way.

I don't have strong feelings either way.  Leave the patch in if it is
your preference.

>>> 5) Add cvs2hg top-level script.
>>> http://vc.gerg.ca/hg/cvs2svn-patches/file/tip/cvs2hg-main.patch
>> This is also not something that was introduced by you, but it seems to
>> me that it would make more sense to move the main() function to file
>> run_options.py (maybe even make it a method of RunOptions?), and to move
>> the foo_main() options to foo_run_options.py (maybe even make them a
>> method of FooRunOptions?)
> 
> Hmmm.  IMHO the duplication between cvs2{svn,git,hg,bzr} is worse than
> the duplication in main.py.

I don't mind the duplication in main.py, which is really quite minimal.
 What I would like to avoid is that cvs2xxx has to import code that is
only useful for cvs2yyy.  Currently, all scripts import main.py, and
main unconditionally imports svn_run_options, git_run_options, and
bzr_run_options, which probably import lots more junk that is
backend-specific.

> (And arguably I've made it worse by
> introducing the Mercurial API version check in cvs2hg, thereby making
> it so the scripts are not trivial copies of each other. Feel free to
> call me on *that*.  ;-)  I'd sooner spend time doing something about
> that duplication, honestly.

I suppose we could have a python_version_check.py module, import it from
each of the main scripts, then call
python_version_check.check_version(progname).  I don't think we want to
move this check to main.py, though, because it has to be loadable from
the widest possible range of Python versions.  (Python files are
executed linearly, but first they are compiled as a whole, so no Python
constructs that are syntax errors under any Python versions can be used
in this file.)

Michael

------------------------------------------------------
http://cvs2svn.tigris.org/ds/viewMessage.do?dsForumId=1667&dsMessageId=2388793

To unsubscribe from this discussion, e-mail: [[email protected]].