Mailing List Archive: Zope: Coders

Question about procedures

Index | Next | Previous | View Threaded

madduck at madduck

Mar 24, 2005, 8:05 AM

Post #1 of 8 (1979 views)
Permalink

Question about procedures

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 [at] madduc

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

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

Attachments:

signature.asc (0.18 KB)

regebro at gmail

Mar 24, 2005, 8:09 AM

Post #2 of 8 (1924 views)
Permalink

Re: Question about procedures [In reply to]

On Thu, 24 Mar 2005 17:05:36 +0100, martin f krafft <madduck [at] madduck> 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 [at] zope
http://mail.zope.org/mailman/listinfo/zope-coders

madduck at madduck

Mar 24, 2005, 8:27 AM

Post #3 of 8 (1906 views)
Permalink

Re: Question about procedures [In reply to]

also sprach Lennart Regebro <regebro [at] gmail> [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 [at] madduc

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

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

Attachments:

signature.asc (0.18 KB)

tseaver at zope

Mar 24, 2005, 8:44 AM

Post #4 of 8 (1873 views)
Permalink

Re: Question about procedures [In reply to]

-----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 [at] zope
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 [at] zope
http://mail.zope.org/mailman/listinfo/zope-coders

fg at nuxeo

Mar 24, 2005, 9:14 AM

Post #5 of 8 (1904 views)
Permalink

Re: Question about procedures [In reply to]

A few notes:

martin f krafft <madduck [at] madduck> 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 [at] nuxeo
_______________________________________________
Zope-Coders mailing list
Zope-Coders [at] zope
http://mail.zope.org/mailman/listinfo/zope-coders

tim at siteworx

Mar 24, 2005, 9:16 AM

Post #6 of 8 (1906 views)
Permalink

Re: Question about procedures [In reply to]

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 [at] zope
http://mail.zope.org/mailman/listinfo/zope-coders

regebro at gmail

Mar 24, 2005, 10:20 AM

Post #7 of 8 (1925 views)
Permalink

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

On Thu, 24 Mar 2005 12:16:36 -0500, Tim McLaughlin <tim [at] siteworx> 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 [at] zope
http://mail.zope.org/mailman/listinfo/zope-coders

lists at andreas-jung

Mar 25, 2005, 12:45 AM

Post #8 of 8 (1923 views)
Permalink

Re: Question about procedures [In reply to]

--On Donnerstag, 24. M�rz 2005 17:27 Uhr +0100 martin f krafft
<madduck [at] madduck> wrote:

> also sprach Lennart Regebro <regebro [at] gmail> [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

Index | Next | Previous | View Threaded

Interested in having your list archived? Contact Gossamer Threads