-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Mathias Weinert wrote:
> I was just starting to test mailer.py and want to provide some
> patches for it (and some questions :-) ).
Thanks!
And, sorry it's taken so long for someone to get to reviewing this fully.
> 1. The script is not very robust against wrong command line
> arguments. You get for example an assertion if you call it with
> a repos path with a trailing slash (like bash completion
> provides it):
> assertion "is_canonical (base, blen)" failed: file "subversion/libsvn_subr/path.c", line 114
> Aborted (core dumped)
>
> But after thinking a while about this I think this might be
> no problem as the script will mostly be used by subversion hooks
> which will always provide the right parameters. On the other hand
> everybody testing the script before implementing it in a hook
> will possibly run into such problems.
> In order to prevent this assertion I changed one line in the
> script: See hunk 6 in patch at bottom of this mail
Thanks, committed with slight enhancement for completeness in r18138.
> 2. It took a while for me to accept that propchange and
> propchange2 are meant to handle revision properties ;-). So why
> not calling these commands revpropchange and revpropchange2? As
> it's probably too late to change the command names now I would
> at least apply the following patch for the command line help:
> See hunks 1 and 5 in patch at bottom of this mail
Thanks, r18139.
> 3. I always run into an error when calling the command 'commit'
> when there are changes in other areas and show_nonmatching_paths
> is set to yes. I always get:
>
> [...]
> Diffs of changes in other areas also in this revision:
> Traceback (most recent call last):
> File "/d/Subversion/subversion-1.3.0-rc4/tools/hook-scripts/mailer/mailer.py", line 1280, in ?
> sys.argv[3:3+expected_args])
> File "/usr/local/lib/svn-python/svn/core.py", line 217, in run_app
> return apply(func, (_core.application_pool,) + args, kw)
> File "/d/Subversion/subversion-1.3.0-rc4/tools/hook-scripts/mailer/mailer.py", line 75, in main
> messenger.generate()
> File "/d/Subversion/subversion-1.3.0-rc4/tools/hook-scripts/mailer/mailer.py", line 364, in generate
> group, params, paths, subpool)
> File "/d/Subversion/subversion-1.3.0-rc4/tools/hook-scripts/mailer/mailer.py", line 621, in generate_content
> renderer.render(data)
> File "/d/Subversion/subversion-1.3.0-rc4/tools/hook-scripts/mailer/mailer.py", line 906, in render
> self._render_diffs(data.other_diffs)
> File "/d/Subversion/subversion-1.3.0-rc4/tools/hook-scripts/mailer/mailer.py", line 941, in _render_diffs
> if not diff.diff and not diff.diff_url:
> AttributeError: DiffGenerator instance has no attribute 'diff'
>
> The cause of this is a typo bug at the generation of other
> diffs: See hunk 2 in patch at bottom of this mail (looks like
> a copy-and-paste-error)
Thanks, r18140.
> 4. Just a small thing when calling the command 'commit' when
> there are changes in other areas and show_nonmatching_paths is
> set to yes. In this case the line 'Diffs of changes in other
> areas also in this revision:' is always printed even if there
> are no changes that match the generate_diffs option. To avoid
> this I moved the writing of this line into _render_diffs (I am
> not a Python programer and I am sure there must be a more
> elegant solution to this): See hunks 3 and 4 in patch at bottom
> of this mail
Thanks, r18141.
I changed names a little to hopefully make the intent of the code clearer.
> 5. If you call mailer.py propchange2 with the wrong action it
> is waiting for user input. I don't know what this is good for
> but am interested in it. Could you please explain me this?
For several actions, Subversion passes some data to a hook script via stdin.
> 6. A question of understanding the configuration file: Let's
> say I am only interested in getting mails about changed .html
> files. So I create a group with 'for_paths = .*\.html'. When
> doing this all .html files are processed with this group and
> all other paths are processed with default. Is there a way
> to explicitly say "do nothing with all other paths" -
> especially if I have some other groups (.c files, .java
> files, ...)? Or do I have to ensure that the default section
> doesn't contain a to_addr for example?
I don't know. I suppose "to_addr = " in a section might work.
Max.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (Cygwin)
iD8DBQFDzaWcfFNSmcDyxYARAhEzAJwK06uOyFvN7k4a/2pew4A+aLlbNwCfcVlF
RklgeR472C8yc2gHZfVsDeg=
=uPy0
-----END PGP SIGNATURE-----
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Jan 18 03:37:22 2006