[RFE] Shall we try to merge my fork?

[leamas]

Here is a list of the changes I have made. The code is now in a reasonable state, with running tests and pilot installation. In my opinion, it's a step forward (from a good start!).

If you are interested, I'm willing to cooperate to get this merged. However, the current history is a mess, so just merging that is a no-go. Also, I don't really expect (or want) you to just accept all these changes. So this message is an invitation to discuss things. If you close it, I will consider the discussion finished.

Once again, if you are interested, my proposal is that you check this list of changes and cross-checks it with the forked code. It guess it would start a discussion about what to merge, what to drop and what to enhance. From that discussion I'll try to make patches, one at a time, which looks reasonable that we can merge into the main repo.

I know it's a lot. I'm perfectly happy if you just pick some pieces as a start. But just sending some commits isn't really the way here IMHO. From my point of view it would make sense to start with the basic issues: style, pylint, threading. But that's me :;)

Compatibility

The changes sketched here are big enough to need a new version IMHO. If we decide to walk this road I suggest leaving current version of plugin maintained as-is while starting a new branch heading for a new version. In this version we drop both GitPython 1.0, old command line commands and git.ini compatibility. A clean cut, so to say. Of course, maintaining some or all of these interfaces is possible, but not worth the cost IMHO.

Style:

• Use pep8-tool to check formatting, given pep8.conf. Forces a general style which is easier to read, verify and agree on.
• Reorder overall plugin: constants, Exceptions, global funcs, private classes and public Git.
• Mark private classes and module functions as private.
• Don't use super(), generally described as evil in these cases.

  Static checking:

• Use pylint, pylint.conf + local pylint disables in the code to check plugin.
• Listen carefully to warnings about e. g., too complex methods or classes, too general exception catching, import clashes etc. and fix those.
• Add missing docstrings
• Remove wildcard imports, generally described as evil in the python community. In this case import supybot.* imports huge namespace with all sorts of collisions, making further static checking (and coding!) harder.

  Logging

• Adapt to the style with a log instance as a object attribute used by most (all?) other plugins. Use log.debug() as appropriate.
• Trust logging module to be reentrant (it is) and do all logging directl in threads.

  Exceptions

• Use a module-specific base exception.
• Don't catch all exceptions unless really motivated e. g., as a outer catch-all in a thread or when closing down(pylint finding).

  Threading

• Drop the @synchronized stuff. Repositiories are locked with the instance lock attribute, making the methods synchronized is just complicated. Means that lock can be simple, non-reentrant.
• Synchronize access to the overall repository list (refactoring needed.)
• Add a callback framework so threads can schedule callbacks in main thread
• Don't run the fetcher thread as a sleep-run long running thread, instead use the scheduler to invoke it periodically. Simpler, no wait loop, more robust on transient errors since a new thread is created each fetch.
• Don't run poll thread as sleep-run, instead let fetcher schedule it to run when something is fetched. Much simpler, minimizes the risks for lock congestion and makes things more responsive (one poll cycle).
• Run git-clone in separate thread (running it in fetch thread makes timeout handling and shorter poll periods harder)
• Add a timer to fetch thread so it doesn't hang on a failing repo.

  Configuration:

• Use supybot's config system for all options, drop git.ini. Pros: Maintenance possible using irc (editing file still OK, though). Cons: A bit awkward handling of multiline items (commitMessage1, commitMessage2)
• Related command changes: new commands to create and destroy repos, sort of falls into place. Need to document how to use @config to handle configuration. rehash is best dropped, since a 'reload git' is needed to pick up new configuration, and might as well be used as rehash as well.

  Multiple branches

• Listen to multiple branches in each repo using a wildcard syntax, '*' is default meaning all branches (see README).
• More complex data when checking what's changed in new_commits_by_branch and when printing.
• branch argument needed in log command, new command to report tracked branches in a repo.

  Refactoring

Some code has been refactored due to reasons such as pylint warnings, threading, multiple branches, commands to create/kill repos etc. Examples:

• The DisplayCtx container handles the somewhat complicated setup data to print commits.
• The Scheduler centralizes scheduling to keep track of it.
• Repository has a private class Options to keep the multitude of options separate from the class' other attributes.
• The _poll method became too big when using multiple branches, thus splitted.
• The Repos object handles the overall repository list which need to be synchronized.

  Command format

Generally depends on other changes, notably using multiple branches and supybot configuration setup. Besides that:

• Keeping the 'log' command is a bad idea IMHO. It's technically complicate but also having a command name so general that it's very likely to clash with other plugins is a bad idea IMHO.
• My solution has been to prefix all commands with 'repo': repolog, repostat,repoadd etc.
• Current command set documented in README.

[mmueller]

@leamas, thanks for taking the time to write this up. I have to be honest, this is more effort than I'm willing to spend on this plugin right now. It seems like you have a vision to make it into something much bigger than what it is. This is essentially a one weekend project that I have maintained because it's useful and it works. For example, I don't see a lot of value in things like validation tools on such a small plugin.

To respond to some specific points:

• I don't see a lot of value in adding tools like pylint to such a small project (essentially a single 500 line file).
• I grappled with the command names, but ultimately I like the names the way they are. Supybot has facilities for dealing with command name clashes (e.g. @git log instead of @log) for a reason.
• I appreciate your threading clean ups and may even want to take some of those changes. But I'm not really interested in spending a lot of time on large-scale refactoring of something that already works (especially since I don't have significant feature development planned...).

This is all a long-winded way of saying you will probably want to maintain a separate fork from me. If everyone seems to prefer your fork over mine, I can abandon mine. Thanks again and I look forward to seeing where your fork is going.