You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Greg Stein <gs...@gmail.com> on 2009/03/31 15:49:01 UTC
Respect for fellow developers (was: svn commit: r36895 ...)
I nearly expounded on this in the log message, but decided that a
regular email to the list is a much better approach. Specifically to
talk about respect for your fellow developers.
When Jane vetoes John's change, then a discussion needs to be started
about the change and what kind of resolution is warranted. Jane does
NOT immediately revert John's change. He may be right after all, or
maybe with some tweaks, the technical problem can be corrected. This
is about showing respect to John to determine the final course of
action. He will let his change stand (Jane removes her veto after some
discussion), he can tweak it (some compromise is reached after
discussion), or he can revert it when he realizes that his fellow
developers disagree with his change and no compromise solution looks
to be available. Jane gives hoim the respect, the time, and the
discussion to take the appropriate course of action.
However, when that third option is reached: the veto stands, even
after discussion, then John MUST respect that decision. This is a
community of peers, and we need to show respect to the opinions,
decisions, and approaches of others. That also means vetoes should not
be casually thrown around: it is a very harsh measure of your peer's
work. In many cases, you may want to consider whether John's change is
"good enough" and just let it stand. Or provide some review on how it
could be improved. Pulling out the veto card is divisive, so Jane
needs to really think about whether the technical problems introduced
by the change warrant her veto.
And with that said, I didn't see the two vetoed revisions getting
backed out as they should have. I wanted to let the original developer
do it, but he was not respecting mine and Branko's issues on the
matter. So with reluctance, I had to do it myself.
Then write this email for why I feel that was wrong.
-g
On Tue, Mar 31, 2009 at 17:24, Greg Stein <gs...@gmail.com> wrote:
> Author: gstein
> Date: Tue Mar 31 08:24:16 2009
> New Revision: 36895
>
> Log:
> Revert r36822 and r36833, due to vetoes.
>
> * subversion/bindings/swig/python/svn/core.py:
> (svn_path_compare_paths): restore use of cmp()
>
> * subversion/tests/cmdline/svntest/tree.py:
> (SVNTreeNode.__cmp__): restored
> (SVNTreeNode.__eq__, SVNTreeNode.__ne__): removed
>
> * subversion/tests/cmdline/svntest/verify.py:
> (ExpectedOutput.__cmp__): restored
> (ExpectedOutput.__eq__, ExpectedOutput.__ne__): removed
> (UnorderedOutput.__cmp__): restored
>
> * tools/dev/contribulyze.py:
> (Contributor): restored (admittedly funky) hash behavior and __cmp__
> and __hash__ methods. removed __eq__, __ne__, __lt__, and __gt__
> methods.
> (LogMessage): restored __cmp__ and __hash__ methods. removed __eq__,
> __ne__, __lt__ and __gt__ methods.
>
> * tools/dev/trails.py:
> (output_summary): restored use of cmp() function
>
> Modified:
> trunk/subversion/bindings/swig/python/svn/core.py
> trunk/subversion/tests/cmdline/svntest/tree.py
> trunk/subversion/tests/cmdline/svntest/verify.py
> trunk/tools/dev/contribulyze.py
> trunk/tools/dev/trails.py
>
> Modified: trunk/subversion/bindings/swig/python/svn/core.py
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/bindings/swig/python/svn/core.py?pathrev=36895&r1=36894&r2=36895
> ==============================================================================
> --- trunk/subversion/bindings/swig/python/svn/core.py Tue Mar 31 07:51:57 2009 (r36894)
> +++ trunk/subversion/bindings/swig/python/svn/core.py Tue Mar 31 08:24:16 2009 (r36895)
> @@ -121,7 +121,7 @@ def svn_path_compare_paths(path1, path2)
>
> # Common prefix was skipped above, next character is compared to
> # determine order
> - return (char1 > char2) - (char1 < char2)
> + return cmp(char1, char2)
>
> def svn_mergeinfo_merge(mergeinfo, changes):
> return _libsvncore.svn_swig_mergeinfo_merge(mergeinfo, changes)
>
> Modified: trunk/subversion/tests/cmdline/svntest/tree.py
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/tests/cmdline/svntest/tree.py?pathrev=36895&r1=36894&r2=36895
> ==============================================================================
> --- trunk/subversion/tests/cmdline/svntest/tree.py Tue Mar 31 07:51:57 2009 (r36894)
> +++ trunk/subversion/tests/cmdline/svntest/tree.py Tue Mar 31 08:24:16 2009 (r36895)
> @@ -276,11 +276,11 @@ class SVNTreeNode:
> return s.getvalue()
>
>
> - def __eq__(self, other):
> - return self.name == other.name
> -
> - def __ne__(self, other):
> - return not self.__eq__(other)
> + def __cmp__(self, other):
> + """Define a simple ordering of two nodes without regard to their full
> + path (i.e. position in the tree). This can be used for sorting the
> + children within a directory."""
> + return cmp(self.name, other.name)
>
> def as_state(self, prefix=None):
> root = self
>
> Modified: trunk/subversion/tests/cmdline/svntest/verify.py
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/tests/cmdline/svntest/verify.py?pathrev=36895&r1=36894&r2=36895
> ==============================================================================
> --- trunk/subversion/tests/cmdline/svntest/verify.py Tue Mar 31 07:51:57 2009 (r36894)
> +++ trunk/subversion/tests/cmdline/svntest/verify.py Tue Mar 31 08:24:16 2009 (r36895)
> @@ -93,7 +93,7 @@ class ExpectedOutput:
> def __str__(self):
> return str(self.output)
>
> - def __eq__(self, other):
> + def __cmp__(self, other):
> """Return whether SELF.output matches OTHER (which may be a list
> of newline-terminated lines, or a single string). Either value
> may be None."""
> @@ -116,12 +116,9 @@ class ExpectedOutput:
> is_match = False
>
> if is_match:
> - return True
> + return 0
> else:
> - return False
> -
> - def __ne__(self, other):
> - return not self.__eq__(other)
> + return 1
>
> def is_equivalent_list(self, expected, actual):
> "Return whether EXPECTED and ACTUAL are equivalent."
> @@ -219,6 +216,14 @@ class RegexOutput(ExpectedOutput):
>
> class UnorderedOutput(ExpectedOutput):
> """Marks unordered output, and performs comparisions."""
> +
> + def __cmp__(self, other):
> + "Handle ValueError."
> + try:
> + return ExpectedOutput.__cmp__(self, other)
> + except ValueError:
> + return 1
> +
> def is_equivalent_list(self, expected, actual):
> "Disregard the order of ACTUAL lines during comparison."
> if self.match_all:
>
> Modified: trunk/tools/dev/contribulyze.py
> URL: http://svn.collab.net/viewvc/svn/trunk/tools/dev/contribulyze.py?pathrev=36895&r1=36894&r2=36895
> ==============================================================================
> --- trunk/tools/dev/contribulyze.py Tue Mar 31 07:51:57 2009 (r36894)
> +++ trunk/tools/dev/contribulyze.py Tue Mar 31 08:24:16 2009 (r36895)
> @@ -122,7 +122,7 @@ def html_footer():
> return '\n</body>\n</html>\n'
>
>
> -class Contributor(object):
> +class Contributor:
> # Map contributor names to contributor instances, so that there
> # exists exactly one instance associated with a given name.
> # Fold names with email addresses. That is, if we see someone
> @@ -131,6 +131,9 @@ class Contributor(object):
> # instance, and store it under both the email and the real name.
> all_contributors = { }
>
> + # See __hash__() for why this is necessary.
> + hash_value = 1
> +
> def __init__(self, username, real_name, email):
> """Instantiate a contributor. Don't use this to generate a
> Contributor for an external caller, though, use .get() instead."""
> @@ -144,6 +147,9 @@ class Contributor(object):
> # "Patch" represent all the revisions for which this contributor
> # contributed a patch.
> self.activities = { }
> + # Sigh.
> + self.unique_hash_value = Contributor.hash_value
> + Contributor.hash_value += 1
>
> def add_activity(self, field_name, log):
> """Record that this contributor was active in FIELD_NAME in LOG."""
> @@ -224,17 +230,20 @@ class Contributor(object):
> else:
> return other_str
>
> - def __eq__(self, other):
> - return self.score() == other.score() and self.big_name() == other.big_name()
> -
> - def __ne__(self, other):
> - return not self.__eq__(other)
> -
> - def __lt__(self, other):
> - return self.score() > other.score() or (self.score() == other.score() and self.big_name() < other.big_name())
> + def __cmp__(self, other):
> + if self.is_full_committer and not other.is_full_committer:
> + return 1
> + if other.is_full_committer and not self.is_full_committer:
> + return -1
> + result = cmp(self.score(), other.score())
> + if result == 0:
> + return cmp(self.big_name(), other.big_name())
> + else:
> + return 0 - result
>
> - def __gt__(self, other):
> - return self.score() < other.score() or (self.score() == other.score() and self.big_name() > other.big_name())
> + def __hash__(self):
> + """See LogMessage.__hash__() for why this exists."""
> + return self.hash_value
>
> @staticmethod
> def parse(name):
> @@ -431,7 +440,7 @@ class Field:
> return s
>
>
> -class LogMessage(object):
> +class LogMessage:
> # Maps revision strings (e.g., "r12345") onto LogMessage instances,
> # holding all the LogMessage instances ever created.
> all_logs = { }
> @@ -459,17 +468,28 @@ class LogMessage(object):
> """Accumulate one more line of raw message."""
> self.message += line
>
> - def __eq__(self, other):
> - return int(self.revision[1:]) == int(other.revision[1:])
> -
> - def __ne__(self, other):
> - return not self.__eq__(other)
> -
> - def __lt__(self, other):
> - return int(self.revision[1:]) > int(other.revision[1:])
> + def __cmp__(self, other):
> + """Compare two log messages by revision number, for sort().
> + Return -1, 0 or 1 depending on whether a > b, a == b, or a < b.
> + Note that this is reversed from normal sorting behavior, but it's
> + what we want for reverse chronological ordering of revisions."""
> + a = int(self.revision[1:])
> + b = int(other.revision[1:])
> + if a > b: return -1
> + if a < b: return 1
> + else: return 0
> +
> + def __hash__(self):
> + """I don't really understand why defining __cmp__() but not
> + __hash__() renders an object type unfit to be a dictionary key,
> + especially in light of the recommendation that if a class defines
> + mutable objects and implements __cmp__() or __eq__(), then it
> + should not implement __hash__(). See these for details:
> + http://mail.python.org/pipermail/python-dev/2004-February/042580.html
> + http://mail.python.org/pipermail/python-bugs-list/2003-December/021314.html
>
> - def __gt__(self, other):
> - return int(self.revision[1:]) < int(other.revision[1:])
> + In the meantime, I think it's safe to use the revision as a hash value."""
> + return int(self.revision[1:])
>
> def __str__(self):
> s = '=' * 15
>
> Modified: trunk/tools/dev/trails.py
> URL: http://svn.collab.net/viewvc/svn/trunk/tools/dev/trails.py?pathrev=36895&r1=36894&r2=36895
> ==============================================================================
> --- trunk/tools/dev/trails.py Tue Mar 31 07:51:57 2009 (r36894)
> +++ trunk/tools/dev/trails.py Tue Mar 31 08:24:16 2009 (r36895)
> @@ -74,9 +74,9 @@ def output_summary(trails, outfile):
> def _freqtable_cmp(a_b, c_d):
> (a, b) = a_b
> (c, d) = c_d
> - c = (d > b) - (d < b)
> + c = cmp(d, b)
> if not c:
> - c = (a > c) - (a < c)
> + c = cmp(a, c)
> return c
>
> def list_frequencies(list):
>
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=495&dsMessageId=1496768
>
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1496987
Re: Respect for fellow developers (was: svn commit: r36895 ...)
Posted by Greg Stein <gs...@gmail.com>.
Nice. And sure, it could be applied to the svn community.
The comment about "being simple"... that's how google's "do no evil"
motto came up. Some people were listing a whole bunch of "do's and
don'ts" on a whiteboard, and I think it was Paul Bucheit who said
something like "um. how about erasing all that just put down 'do no
evil'?".
Cheers,
-g
On Wed, Apr 1, 2009 at 12:56, Gavin 'Beau' Baumanis
<ga...@thespidernet.com> wrote:
> +1
>
> Perhaps there's scope for this (advice) to be included in HACKING?
>
> Of course, nothing to do with THIS issue, but none the less I feel a worthy
> message for new community members and the experienced, alike.
>
> I remember back in the days of dial-up BBS's the posting rules contained
> this "gem";
>
> (paraphrasing)
> Do Not excessively annoy other.
> Do not allow yourself to be excessively annoyed.
>
> These two very simple rules seemed to keep nearly all "irritations" at an
> acceptable level.
>
> And while those two aren't necessarily applicable to the SVN community - I
> bring them to the lists' attention as an illustration of how something
> "simple" need only be, to be effective.
>
> Beau.
>
>
> On 01/04/2009, at 3:00 AM, C. Michael Pilato wrote:
>
>> Greg Stein wrote:
>>>
>>> I nearly expounded on this in the log message, but decided that a
>>> regular email to the list is a much better approach. Specifically to
>>> talk about respect for your fellow developers.
>>>
>>> When Jane vetoes John's change, then a discussion needs to be started
>>> about the change and what kind of resolution is warranted. Jane does
>>> NOT immediately revert John's change. He may be right after all, or
>>> maybe with some tweaks, the technical problem can be corrected. This
>>> is about showing respect to John to determine the final course of
>>> action. He will let his change stand (Jane removes her veto after some
>>> discussion), he can tweak it (some compromise is reached after
>>> discussion), or he can revert it when he realizes that his fellow
>>> developers disagree with his change and no compromise solution looks
>>> to be available. Jane gives hoim the respect, the time, and the
>>> discussion to take the appropriate course of action.
>>>
>>> However, when that third option is reached: the veto stands, even
>>> after discussion, then John MUST respect that decision. This is a
>>> community of peers, and we need to show respect to the opinions,
>>> decisions, and approaches of others. That also means vetoes should not
>>> be casually thrown around: it is a very harsh measure of your peer's
>>> work. In many cases, you may want to consider whether John's change is
>>> "good enough" and just let it stand. Or provide some review on how it
>>> could be improved. Pulling out the veto card is divisive, so Jane
>>> needs to really think about whether the technical problems introduced
>>> by the change warrant her veto.
>>>
>>> And with that said, I didn't see the two vetoed revisions getting
>>> backed out as they should have. I wanted to let the original developer
>>> do it, but he was not respecting mine and Branko's issues on the
>>> matter. So with reluctance, I had to do it myself.
>>>
>>> Then write this email for why I feel that was wrong.
>>>
>>> -g
>>
>> Greg, this is a good reminder to all about the kind of fundamental, mutual
>> respect that makes this community function at all.
>>
>> I would further admonish folks to be aware of the danger of ignoring the
>> small fissures that can form in relationships -- especially remote ones
>> such
>> as we mostly have represented in our community -- and to proactively work
>> to
>> repair those matters privately before deeper damage is done. (And to do
>> so
>> using whatever medium is required for clear communication). Life is far
>> too
>> short, and our individual pride far too valueless, to permit discord
>> amongst
>> each other.
>>
>> --
>> C. Michael Pilato <cm...@collab.net>
>> CollabNet <> www.collab.net <> Distributed Development On Demand
>>
>> ------------------------------------------------------
>>
>> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1497158
>
>
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1506376
Re: Respect for fellow developers (was: svn commit: r36895 ...)
Posted by Gavin Baumanis <ga...@thespidernet.com>.
+1
Perhaps there's scope for this (advice) to be included in HACKING?
Of course, nothing to do with THIS issue, but none the less I feel a
worthy message for new community members and the experienced, alike.
I remember back in the days of dial-up BBS's the posting rules
contained this "gem";
(paraphrasing)
Do Not excessively annoy other.
Do not allow yourself to be excessively annoyed.
These two very simple rules seemed to keep nearly all "irritations" at
an acceptable level.
And while those two aren't necessarily applicable to the SVN community
- I bring them to the lists' attention as an illustration of how
something "simple" need only be, to be effective.
Beau.
On 01/04/2009, at 3:00 AM, C. Michael Pilato wrote:
> Greg Stein wrote:
>> I nearly expounded on this in the log message, but decided that a
>> regular email to the list is a much better approach. Specifically to
>> talk about respect for your fellow developers.
>>
>> When Jane vetoes John's change, then a discussion needs to be started
>> about the change and what kind of resolution is warranted. Jane does
>> NOT immediately revert John's change. He may be right after all, or
>> maybe with some tweaks, the technical problem can be corrected. This
>> is about showing respect to John to determine the final course of
>> action. He will let his change stand (Jane removes her veto after
>> some
>> discussion), he can tweak it (some compromise is reached after
>> discussion), or he can revert it when he realizes that his fellow
>> developers disagree with his change and no compromise solution looks
>> to be available. Jane gives hoim the respect, the time, and the
>> discussion to take the appropriate course of action.
>>
>> However, when that third option is reached: the veto stands, even
>> after discussion, then John MUST respect that decision. This is a
>> community of peers, and we need to show respect to the opinions,
>> decisions, and approaches of others. That also means vetoes should
>> not
>> be casually thrown around: it is a very harsh measure of your peer's
>> work. In many cases, you may want to consider whether John's change
>> is
>> "good enough" and just let it stand. Or provide some review on how it
>> could be improved. Pulling out the veto card is divisive, so Jane
>> needs to really think about whether the technical problems introduced
>> by the change warrant her veto.
>>
>> And with that said, I didn't see the two vetoed revisions getting
>> backed out as they should have. I wanted to let the original
>> developer
>> do it, but he was not respecting mine and Branko's issues on the
>> matter. So with reluctance, I had to do it myself.
>>
>> Then write this email for why I feel that was wrong.
>>
>> -g
>
> Greg, this is a good reminder to all about the kind of fundamental,
> mutual
> respect that makes this community function at all.
>
> I would further admonish folks to be aware of the danger of ignoring
> the
> small fissures that can form in relationships -- especially remote
> ones such
> as we mostly have represented in our community -- and to proactively
> work to
> repair those matters privately before deeper damage is done. (And
> to do so
> using whatever medium is required for clear communication). Life is
> far too
> short, and our individual pride far too valueless, to permit discord
> amongst
> each other.
>
> --
> C. Michael Pilato <cm...@collab.net>
> CollabNet <> www.collab.net <> Distributed Development On
> Demand
>
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1497158
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1505814
Re: Respect for fellow developers (was: svn commit: r36895 ...)
Posted by "C. Michael Pilato" <cm...@collab.net>.
Greg Stein wrote:
> I nearly expounded on this in the log message, but decided that a
> regular email to the list is a much better approach. Specifically to
> talk about respect for your fellow developers.
>
> When Jane vetoes John's change, then a discussion needs to be started
> about the change and what kind of resolution is warranted. Jane does
> NOT immediately revert John's change. He may be right after all, or
> maybe with some tweaks, the technical problem can be corrected. This
> is about showing respect to John to determine the final course of
> action. He will let his change stand (Jane removes her veto after some
> discussion), he can tweak it (some compromise is reached after
> discussion), or he can revert it when he realizes that his fellow
> developers disagree with his change and no compromise solution looks
> to be available. Jane gives hoim the respect, the time, and the
> discussion to take the appropriate course of action.
>
> However, when that third option is reached: the veto stands, even
> after discussion, then John MUST respect that decision. This is a
> community of peers, and we need to show respect to the opinions,
> decisions, and approaches of others. That also means vetoes should not
> be casually thrown around: it is a very harsh measure of your peer's
> work. In many cases, you may want to consider whether John's change is
> "good enough" and just let it stand. Or provide some review on how it
> could be improved. Pulling out the veto card is divisive, so Jane
> needs to really think about whether the technical problems introduced
> by the change warrant her veto.
>
> And with that said, I didn't see the two vetoed revisions getting
> backed out as they should have. I wanted to let the original developer
> do it, but he was not respecting mine and Branko's issues on the
> matter. So with reluctance, I had to do it myself.
>
> Then write this email for why I feel that was wrong.
>
> -g
Greg, this is a good reminder to all about the kind of fundamental, mutual
respect that makes this community function at all.
I would further admonish folks to be aware of the danger of ignoring the
small fissures that can form in relationships -- especially remote ones such
as we mostly have represented in our community -- and to proactively work to
repair those matters privately before deeper damage is done. (And to do so
using whatever medium is required for clear communication). Life is far too
short, and our individual pride far too valueless, to permit discord amongst
each other.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1497158