Re: [Framework-Team] PLIP #126 ready for review
> * Running the tests of plone.app.controlpanel on a vanilla checkout of the > plip buildout generated one failure for me (Ubuntu/packaged uncustomized > Python 2.4): > > ... > > Set up Products.PloneTestCase.layer.PloneSite in 18.665 seconds. > Running: > . > > Failure in test > /home/ritz/buildouts/reviewing/plip126-link-redirects/src/plone.app.controlpanel/plone/app/controlpanel/tests/types.txt > Failed doctest test for types.txt > File > "/home/ritz/buildouts/reviewing/plip126-link-redirects/src/plone.app.controlpanel/plone/app/controlpanel/tests/types.txt", > line 0 > > -- > File > "/home/ritz/buildouts/reviewing/plip126-link-redirects/src/plone.app.controlpanel/plone/app/controlpanel/tests/types.txt", > line 75, in types.txt > Failed example: > self.browser.contents > Expected: > '...Globally addable... > ...Allow comments... > ...Visible in searches... > ... ...type="checkbox"... > ...name="redirect_links:boolean"... > ...checked="checked">... > .. > ...Redirect links to remote url...' > Got: > '\n \n\n\n Transitional//EN"\n > "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd";>\n\n\n xmlns="http://www.w3.org/1999/xhtml"; xml:lang="en"\n lang="en">\n\n > (truncated by me here) > > Guess it's because it expects ...Redirect links to remote url...' but > it gets ...Redirect immediately to link target... > Just for the record, this was broken for a brief time on our branch due to some jostling around of language, but was resolved in the following changeset: http://dev.plone.org/plone/changeset/24272 I hope that lies w/in your "as of 2009-01-16" range (e.g. it was fixed right after you started your review). Not sure though. I'm not seeing a failure right now at least. I can look at the CMFPlone failures now. Andrew ___ Framework-Team mailing list Framework-Team@lists.plone.org http://lists.plone.org/mailman/listinfo/framework-team
Re: [Framework-Team] PLIP 237 ready for review
On Jan 18, 2009, at 2:44 PM, Hanno Schlichting wrote: I'm somewhat late with creating the review buildout for PLIP 237 (Minor i18n upgrades) at: https://svn.plone.org/svn/plone/review/plip237-minor-i18n-upgrades/ The review notes and detailed explanation is found in the review buildout's README.txt. i've added notes in http://dev.plone.org/plone/changeset/24772 in short: i'm +1 for merging, but PLT has an error which needs to be fixed before the next release. it's not related to the changes, though. cheers, andi -- zeidler it consulting - http://zitc.de/ - i...@zitc.de friedelstraße 31 - 12047 berlin - telefon +49 30 25563779 pgp key at http://zitc.de/pgp - http://wwwkeys.de.pgp.net/ plone 3.2rc1 released! -- http://plone.org/products/plone/ PGP.sig Description: This is a digitally signed message part ___ Framework-Team mailing list Framework-Team@lists.plone.org http://lists.plone.org/mailman/listinfo/framework-team
Re: [Framework-Team] PLIP #247 ready for review (I think)
On Fri, Jan 30, 2009 at 21:00, Ethan Jucovy wrote: > * after seeing your reply, I tried running the tests in the PLIP buildout's > environment with `bin/instance test -s z3c.autoinclude` and did see failures > * i've now fixed them Yay! :-) > * can you reconfirm that the failures you saw were from utils.txt? the ones > i saw were from plugin.txt (though the assertion that was triggered was > indeed the alarming one in utils.py) > * briefly, the problem was that the "foo" module in the demonstration > package i provided collided with another "foo" module that's installed in > z3c.autoinclude's test environment > (z3c.autoinclude/src/z3c/autoinclude/tests/FooPackage is the egg). moral: > namespaces are indeed a good idea. :) i've renamed the demo package to > plip247.foo and will also namespace z3c.autoinclude's test packages before > the next release so this doesn't happen again I've updated the foo.cfg buildout config to reflect this. > * the reason that assertion was so alarming and vague is because, when i > wrote the code, i had no idea what sort of situation could possibly trigger > it. so i figured i might as well throw in an assertion, even if // because i > couldn't imagine what its failure would mean. now that it's actually popped > up i think i understand it: it's asserting that no more than one > non-empty-virtual-namespace-module named "foo" (for any string "foo") will > be importable simultaneously. i'll make the assertion less alarming and > more informative as soon as i figure out a concise way to say that. Great! It's a good idea to explicitly warn about this. > * do the tests pass for you now in a fresh buildout // if you svn up and > re-install the "foo" demo package? again, they are passing for me now in > this environment, let me know if you're seeing different failures. The tests now all pass, I've upgraded my review verdict to a +1. :-) > Thanks again for taking the time to look at this, I really appreciate all of > your notes (and the buildout help!) & I'll try to be as responsive as I > possibly can. np! Thanks for your hard work on implementing the PLIP! -- Martijn Pieters ___ Framework-Team mailing list Framework-Team@lists.plone.org http://lists.plone.org/mailman/listinfo/framework-team
Re: [Framework-Team] PLIP Tallies
Previously Tom Lazar wrote: > FYI i have completed my two remaining reviews and have additionally > reviewed #243 which was without a review. I've fixed the problems you saw with #243. Might I request that review comments are emailed to the PLIP author? Wichert. -- Wichert Akkerman It is simple to make things. http://www.wiggy.net/ It is hard to make things simple. ___ Framework-Team mailing list Framework-Team@lists.plone.org http://lists.plone.org/mailman/listinfo/framework-team
Re: [Framework-Team] PLIP Tallies
On Fri, Jan 30, 2009 at 20:47, Tom Lazar wrote: > there are still outstanding reviews from raphael, witsch and mj (or have > they perhaps not updated the pliptallies page[1]?) and IMO #243 would also > warrant a UI review! Hopefully I'll have some time tomorrow to finish the RR enhancements review; I've had a unexpected long day today and am exhausted. -- Martijn Pieters ___ Framework-Team mailing list Framework-Team@lists.plone.org http://lists.plone.org/mailman/listinfo/framework-team
Re: [Framework-Team] PLIP #247 ready for review (I think)
I just spent three hours writing a detailed email explaining the situation, but gmail seems to have completely swallowed it, drafts and all, when I hit "send". I'm travelling right now and have very limited internet connectivity here, so I'm just going to try to briefly summarize my original reply; if further clarification is needed, let me know and I'll provide more information as soon as I can. Sorry to be so terse, this gmail accident is just making me *really* grumpy. :) * the test failures are on the trunk of z3c.autoinclude; releases have been stable and are tested on multiple OSes and python environments before I cut them. i'll continue to be careful about this in the future. * the tests pass (for me at least) in the environment created by z3c.autoinclude's standalone buildout -- which is the only way I thought to run them before submitting the plip * after seeing your reply, I tried running the tests in the PLIP buildout's environment with `bin/instance test -s z3c.autoinclude` and did see failures * i've now fixed them * can you reconfirm that the failures you saw were from utils.txt? the ones i saw were from plugin.txt (though the assertion that was triggered was indeed the alarming one in utils.py) * briefly, the problem was that the "foo" module in the demonstration package i provided collided with another "foo" module that's installed in z3c.autoinclude's test environment (z3c.autoinclude/src/z3c/autoinclude/tests/FooPackage is the egg). moral: namespaces are indeed a good idea. :) i've renamed the demo package to plip247.foo and will also namespace z3c.autoinclude's test packages before the next release so this doesn't happen again * the reason that assertion was so alarming and vague is because, when i wrote the code, i had no idea what sort of situation could possibly trigger it. so i figured i might as well throw in an assertion, even if // because i couldn't imagine what its failure would mean. now that it's actually popped up i think i understand it: it's asserting that no more than one non-empty-virtual-namespace-module named "foo" (for any string "foo") will be importable simultaneously. i'll make the assertion less alarming and more informative as soon as i figure out a concise way to say that. * do the tests pass for you now in a fresh buildout // if you svn up and re-install the "foo" demo package? again, they are passing for me now in this environment, let me know if you're seeing different failures. Thanks again for taking the time to look at this, I really appreciate all of your notes (and the buildout help!) & I'll try to be as responsive as I possibly can. thanks, ethan On Sun, Jan 25, 2009 at 9:31 AM, Martijn Pieters wrote: > On Sat, Jan 17, 2009 at 02:37, Ethan Jucovy > wrote: > > I've implemented PLIP #247, Automate ZCML Loading for Plone Plugins > > (http://plone.org/products/plone/roadmap/247) and committed a review > > buildout here: > > https://svn.plone.org/svn/plone/review/plip247-automate-plugin-zcml > > for your review. > > I have started my review. > > To my dismay, however, the z3c.autoinclude package has several test > failures. I understand that this package is also being used by the > Grok project, and it appears to work as advertised, but test failures > is not a good sign of package quality. For example, utils.txt somehow > triggers an assert on line 99 in utils.py, which has a somewhat > distressing comment: > > assert len(non_namespaced_dists) == 1 ### we really are in > trouble if we get into a situation with more than one > > Could you comment on the state of these failures? > > > The buildout is branched from the 3.3 review bundle > > buildout based on buildout.eggtractor, which automatically generates ZCML > > slugs for development packages and puts them in > parts/instance/etc/package- > > includes. Since the purpose of PLIP 247 is to automate ZCML loading, > those > > automatically generated ZCML slugs should be removed. I've hardly ever > used > > buildout so I'm not sure how to change that behavior -- sorry about that; > > I'll change it if I can figure out how. > > Simply *not* use buildout.eggtractor; you only have 3 eggs in this > buildout, add them to the develop, eggs and zcml lines and Bob's your > uncle. Alternatively, just set tractor-autoload-zcml to false, which > you did. > > -- > Martijn Pieters > ___ Framework-Team mailing list Framework-Team@lists.plone.org http://lists.plone.org/mailman/listinfo/framework-team
Re: [Framework-Team] PLIP Tallies
FYI i have completed my two remaining reviews and have additionally reviewed #243 which was without a review. there are still outstanding reviews from raphael, witsch and mj (or have they perhaps not updated the pliptallies page[1]?) and IMO #243 would also warrant a UI review! i have included all reviews inside the bundle with a REVIEW-NOTES.txt as per convention and have additionally posted my review as a comment at the plip page. i have not mailed the authors or this list with each individual review. that's all for now, gotta run! cheers, tom [1] https://dev.plone.org/plone/wiki/PLIPTallies33 On 29.01.2009, at 10:49, Andreas Zeidler wrote: hi framework team, we're quickly approaching the deadline for the first round of reviews. most PLIPs have already seen at least one review, but in total we're still at only 11 out of 24 necessary bundle reviews. we're also still one UI review short. so let's push a little harder and try to get those reviews done! :) in related matters... On Jan 19, 2009, at 4:36 PM, Andreas Zeidler wrote: On Jan 19, 2009, at 3:58 PM, Martijn Pieters wrote: Only 2 plips do not yet have a second reviewer assigned: #237 "Minor i18n upgrades" #243 "Replace workflow history viewlet with content history viewlet" Shall we tackle these ad hoc or will someone take these? perhaps let's get going with what's assigned and wait for any of the three missing PLIPs to potentially be submitted before taking care of the remaining review, shall we? i think it's time to assign those now, mostly so that nobody gets the impression they're already done or almost... ;) with a total of 12 PLIPs we get to review six each, so everybody needs to sign up for one more. i just did so (for #237) and would like to ask you to fill in the remaining slots today. also, raphael, i saw you sent some comments, but could you please update the overview page[*] as well, so we get a better, well, overview. :) cheers, andi [*] according to https://dev.plone.org/plone/wiki/PLIPTallies33 -- zeidler it consulting - http://zitc.de/ - i...@zitc.de friedelstraße 31 - 12047 berlin - telefon +49 30 25563779 pgp key at http://zitc.de/pgp - http://wwwkeys.de.pgp.net/ plone 3.1.7 released! -- http://plone.org/products/plone/ ___ Framework-Team mailing list Framework-Team@lists.plone.org http://lists.plone.org/mailman/listinfo/framework-team
