Re: Jens, please try to get more peer review

Hi Stefan,

On Thu, May 29, 2008 at 11:19:48AM +0200, Stefan Sperling wrote:
> you have made a few commits outside the area that you have
> been nominated a partial committer for (German translations).

ahm, yes. But really not far outside :-)

> This is great, I am very happy to see you contributing to
> Subversion even more than by translating -- which is
> already quite some job! I've tried updating the German
> translation before, and it took me ages, so I was very glad
> when you sent another update in time for 1.5 :)

Now I expect that you find and report errors in it :-)

> But I think that some of your commits made outside the German
> translation should have gotten more peer review. Specifically,
> you have never put an 'Approved-by:' line in any of your commits
> outside of your area (r31457, r31459, r31460, r31461, r31484,
> r31491, and r31496).

As you noticed, most are just trivial 'obvious fixes' ...
r31457: TRANSLATING: Removed broken links.

r31459: Added svn:executable flags to various Python files.
r31460: Added svn:executable flags to various Shell files.

Daniel Shahaf added +1 in
http://subversion.tigris.org/servlets/ReadMsg?list=dev&msgNo=139124
and mentioned it a "obvious fix" which doesn't require approval.

Should I add "Approved-by:" to this kind of patches?

To be honest, what could I made wrong with such a commit? I even
did not add additional Shebang lines to qualify for more executable
flags ... Are there volunteers to do this?

r31461: Removed a useless space in front of \n in a message.

r31484: Use workaround for an xgettext limitation.
r31496: Marked a further string explicitly translatable which is also
missed by xgettext.

It affected my translations stats and I posted it to the list first :-)
In summary it just added a macro which expands to an empty string
and few comments. In the 1.5.x branch I just modified all PO files
to get this fixed.

r31491: Fixed a build error for Windows introduced in r31484.

If I make an error I have to correct it :-)
Erik Huelsmann explained how to fix it but gave no explicit approval.

> Why not?

Most of the time because the 'obvious fix'. If there was any doubt
I also wrote to this list. I also just forgot adding this flag at
least once.

> I know that some of the changes you made fall under the 'obvious
> fix' rule, but please keep in mind that the definition of 'obvious fix'
> varies from person to person.

OK.

> Another note on 'Approved-by': I think it helped me a lot to
> always have my changes reviewed while I was a partial committer
> committing outside my area.

If I have any doubt I would never hesitate to ask.

> Please try to get your commits outside the German translation
> approved in the future.

OK, will do so.

Thanks,
Jens

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-05-29 16:27:19 CEST