Mailing List Archive: Question about procedures

Question about procedures

Mar 24, 2005, 8:05 AM

Post #1 of 8 (3825 views)

Dear coders,

I am new; I just got my account today and am thus a little shakey
wrt policies and such. Thus, let me ask you with respect to
a specific example whether I can just commit to the trunk like that:

The following patch is a follow-up to the one I submitted to the
collector yesterday (331). Am I allowed to commit patches of this
nature "just like that" (given proper testing and the like), or
should these be discussed up front and/or committed to my own branch
instead?

Index: TypesTool.py
===================================================================
RCS file: /cvs-repository/Products/CMFCore/TypesTool.py,v
retrieving revision 1.85
diff -u -r1.85 TypesTool.py
--- TypesTool.py 23 Mar 2005 21:56:09 -0000 1.85
+++ TypesTool.py 24 Mar 2005 15:56:08 -0000
@@ -862,12 +862,14 @@

ob = info.constructInstance(container, id, *args, **kw)

- if RESPONSE is not None:
+ if RESPONSE is not None and ob:
+ if not hasattr(ob, 'absolute_url'):
+ raise TypeError('constructInstance did not return a CMF object.')
immediate_url = '%s/%s' % ( ob.absolute_url()
, info.immediate_view )
RESPONSE.redirect( immediate_url )

- return ob.getId()
+ return getattr(ob, 'id', None)

security.declarePrivate( 'listActions' )
def listActions(self, info=None, object=None):

--
martin; (greetings from the heart of the sun.)
\____ echo mailto: !#^."<*>"|tr "<*> mailto:" net@madduck

invalid/expired pgp subkeys? use subkeys.pgp.net as keyserver!
spamtraps: madduck.bogus@madduck.net

no cat has eight tails.
a cat has one tail more than no cat.
therefore, a cat has nine tails.

Re: Question about procedures [ In reply to ]

regebro at gmail

Mar 24, 2005, 8:09 AM

Post #2 of 8 (3744 views)

Permalink

On Thu, 24 Mar 2005 17:05:36 +0100, martin f krafft <madduck@madduck.net> wrote:
> The following patch is a follow-up to the one I submitted to the
> collector yesterday (331). Am I allowed to commit patches of this
> nature "just like that" (given proper testing and the like), or
> should these be discussed up front and/or committed to my own branch
> instead?

Both. Discuss them (zope-dev is the standard place) and make a branch.
--
Lennart Regebro, Nuxeo http://www.nuxeo.com/
CPS Content Management http://www.cps-project.org/
_______________________________________________
Zope-Coders mailing list
Zope-Coders@zope.org
http://mail.zope.org/mailman/listinfo/zope-coders

Re: Question about procedures [ In reply to ]

madduck at madduck

Mar 24, 2005, 8:27 AM

Post #3 of 8 (3725 views)

Permalink

also sprach Lennart Regebro <regebro@gmail.com> [2005.03.24.1709 +0100]:
> Both. Discuss them (zope-dev is the standard place) and make a branch.

I agree with this approach, but isn't it complete overkill for such
small fixes?

--
martin; (greetings from the heart of the sun.)
\____ echo mailto: !#^."<*>"|tr "<*> mailto:" net@madduck

invalid/expired pgp subkeys? use subkeys.pgp.net as keyserver!
spamtraps: madduck.bogus@madduck.net

"even if you persuade me, you won't persuade me."
-- aristophanes

Re: Question about procedures [ In reply to ]

tseaver at zope

Mar 24, 2005, 8:44 AM

Post #4 of 8 (3701 views)

Permalink

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

martin f krafft wrote:
| Dear coders,
|
| I am new; I just got my account today and am thus a little shakey
| wrt policies and such. Thus, let me ask you with respect to
| a specific example whether I can just commit to the trunk like that:
|
| The following patch is a follow-up to the one I submitted to the
| collector yesterday (331). Am I allowed to commit patches of this
| nature "just like that" (given proper testing and the like), or
| should these be discussed up front and/or committed to my own branch
| instead?

Depends. If you are fixing an obvious typo, then go ahead and commit,
after ensuring that all tests pass. If you are changing behavior in a
non-backwards-compatible way (which the first part of your patch does),
then you likely need either a collector issue (to document the
rationale), or some discussion / consensus on the list, which might then
result in a collector issue.

The right list for CMF-specific changes is the zope-cmf list (there is
no separate 'dev' list for CMF).

| Index: TypesTool.py
| ===================================================================
| RCS file: /cvs-repository/Products/CMFCore/TypesTool.py,v
| retrieving revision 1.85
| diff -u -r1.85 TypesTool.py
| --- TypesTool.py 23 Mar 2005 21:56:09 -0000 1.85
| +++ TypesTool.py 24 Mar 2005 15:56:08 -0000
| @@ -862,12 +862,14 @@
|
| ob = info.constructInstance(container, id, *args, **kw)
|
| - if RESPONSE is not None:
| + if RESPONSE is not None and ob:
| + if not hasattr(ob, 'absolute_url'):
| + raise TypeError('constructInstance did not return a CMF
object.')
| immediate_url = '%s/%s' % ( ob.absolute_url()
| , info.immediate_view )
| RESPONSE.redirect( immediate_url )
|
| - return ob.getId()
| + return getattr(ob, 'id', None)
|
| security.declarePrivate( 'listActions' )
| def listActions(self, info=None, object=None):
|

Tres.
- --
===============================================================
Tres Seaver tseaver@zope.com
Zope Corporation "Zope Dealers" http://www.zope.com
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.4 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFCQu6KGqWXf00rNCgRAo/HAKCU4jTMmIRC9Oc3nvY/Be/tNVwatgCcCF0w
XSaQekYrk3pNc0ybN1AUrGc=
=dKdU
-----END PGP SIGNATURE-----

_______________________________________________
Zope-Coders mailing list
Zope-Coders@zope.org
http://mail.zope.org/mailman/listinfo/zope-coders

Re: Question about procedures [ In reply to ]

fg at nuxeo

Mar 24, 2005, 9:14 AM

Post #5 of 8 (3731 views)

Permalink

A few notes:

martin f krafft <madduck@madduck.net> wrote:
> Index: TypesTool.py
> ===================================================================
> RCS file: /cvs-repository/Products/CMFCore/TypesTool.py,v
> retrieving revision 1.85
> diff -u -r1.85 TypesTool.py
> --- TypesTool.py 23 Mar 2005 21:56:09 -0000 1.85
> +++ TypesTool.py 24 Mar 2005 15:56:08 -0000
> @@ -862,12 +862,14 @@
>
> ob = info.constructInstance(container, id, *args, **kw)
>
> - if RESPONSE is not None:
> + if RESPONSE is not None and ob:

You should check 'and ob is not None' too.
But why could it be None ? What's the point (sorry I don't have context).

> + if not hasattr(ob, 'absolute_url'):

Do not use hasattr for persistent objects. Use
if getattr(ob, 'absolute_url', None) is None:

> + raise TypeError('constructInstance did not return a CMF object.')

Also, check your indentation (should be 4 chars).

> immediate_url = '%s/%s' % ( ob.absolute_url()
> , info.immediate_view )
> RESPONSE.redirect( immediate_url )
>
> - return ob.getId()
> + return getattr(ob, 'id', None)

Please don't do that, getId() is the proper API to call.

Florent

--
Florent Guillaume, Nuxeo (Paris, France) CTO, Director of R&D
+33 1 40 33 71 59 http://nuxeo.com fg@nuxeo.com
_______________________________________________
Zope-Coders mailing list
Zope-Coders@zope.org
http://mail.zope.org/mailman/listinfo/zope-coders

Re: Question about procedures [ In reply to ]

tim at siteworx

Mar 24, 2005, 9:16 AM

Post #6 of 8 (3726 views)

Permalink

What should we do if no discussion ensues on zope-dev? Go ahead and
make the commit to the trunk if it's backwards compatible?

Thanks,
Tim

martin f krafft wrote:
| Dear coders,
|
| I am new; I just got my account today and am thus a little shakey wrt
| policies and such. Thus, let me ask you with respect to a specific
| example whether I can just commit to the trunk like that:
|
| The following patch is a follow-up to the one I submitted to the
| collector yesterday (331). Am I allowed to commit patches of this
| nature "just like that" (given proper testing and the like), or should

| these be discussed up front and/or committed to my own branch instead?

Depends. If you are fixing an obvious typo, then go ahead and commit,
after ensuring that all tests pass. If you are changing behavior in a
non-backwards-compatible way (which the first part of your patch does),
then you likely need either a collector issue (to document the
rationale), or some discussion / consensus on the list, which might then
result in a collector issue.

--
Tim McLaughlin
Chief Technology Officer
Siteworx, Inc. Innovative internet solutions.
703.964.0300 ext. 208
_______________________________________________
Zope-Coders mailing list
Zope-Coders@zope.org
http://mail.zope.org/mailman/listinfo/zope-coders

Re: Re: Question about procedures [ In reply to ]

regebro at gmail

Mar 24, 2005, 10:20 AM

Post #7 of 8 (3747 views)

Permalink

On Thu, 24 Mar 2005 12:16:36 -0500, Tim McLaughlin <tim@siteworx.com> wrote:
> What should we do if no discussion ensues on zope-dev? Go ahead and
> make the commit to the trunk if it's backwards compatible?

Yeah, sure. If you think the change should be there, if it doens't
break anything, and nobody protests, then it's probably fine.

One of the reasons why using branches even for small fixes (well, not
typos or other completely obvious things) is that it's easy to undo if
it's bad. When the merge to the trunk is in one go, it's easy to undo.
(And trust me, even if you think that your fix is a simple "one commit
only" it quite often turns out to not be. Branches are good. )

The second reason is that it's easy to merge the fix into other
branches, for example you might to commit it both to trunk and to the
1.4 branch.

So for everything above simple typos, use a branch, make a test that
shows the bug, fix the bug, run the tests, test the changes yourself
for a couple of days or so, and then merge it.

That way you make one commit where it's easy to see who fixed it, what
they fixed and why, it's easy to apply it to other brances, and in
worst case, it's easy to undo.

--
Lennart Regebro, Nuxeo http://www.nuxeo.com/
CPS Content Management http://www.cps-project.org/
_______________________________________________
Zope-Coders mailing list
Zope-Coders@zope.org
http://mail.zope.org/mailman/listinfo/zope-coders

Re: Question about procedures [ In reply to ]

lists at andreas-jung

Mar 25, 2005, 12:45 AM

Post #8 of 8 (3742 views)

Permalink

--On Donnerstag, 24. März 2005 17:27 Uhr +0100 martin f krafft
<madduck@madduck.net> wrote:

> also sprach Lennart Regebro <regebro@gmail.com> [2005.03.24.1709 +0100]:
>> Both. Discuss them (zope-dev is the standard place) and make a branch.
>
> I agree with this approach, but isn't it complete overkill for such
> small fixes?
>

For changes which are limited to a file or a subtree I do always prefer a
patch instead of a branch.
It is easier to handle a patch than a branch. If a branch with the patch
and the destination branch differ too much
it can be very hard to merge both and to figure out the important
difference. Therefore a patch often can be applied
easier to "newer" code even if the patch has been made against an older
code base...but it really depends
from case to case.

-aj

Mailing List Archive

Attached Files:

Attached Files: