Discussion:
[Framework-Team] PLIP 20144
Philip Bauer
2014-12-01 12:13:13 UTC
Permalink
Dear Framework-team,

I got the message that https://dev.plone.org/ticket/20144 should be moved to 5.1, which is generally ok by me but I can't help wondering why.

The implementation, upgrade-steps and tests are all done. Working with folderish dexterity-types in real projects has been tested by many developers for quite some time. What do you think needs more work or improvements for it to be included in Plone 5? I even migrated the test for plone.app.contentmenu (https://github.com/plone/plone.app.contentmenu/pull/8) to work with AT and DX to make sure nothing changes regarding the UI.

I'm more than willing to put in more time if it means having this in Plone 5.0 and not 5.1 (which will be at least a year form now). I think the point when we move from AT to DX is also the right moment to switch from itemish to folderish.

Also: If the -1 stands: You did not answer the question about the alternative folderish profile.

Philip

--
Starzel.de
Philip Bauer
Adlzreiterstr. 35
80337 München
Tel: 089 - 189 29 533
Fax: 089 - 189 29 535
***@starzel.de
www.starzel.de
Johannes Raggam
2014-12-01 12:55:46 UTC
Permalink
The decision was made because we don't want to have Plone 5 beta further
delayed and some were opting for a feature freeze. The changes in PLIP
20144 might be small and the positive impact big, but there could be
some negative impact: UI wise (although the default behavior isn't
changed), performance wise (you already proofed, that tests run the same
speed with or without folderish base types) or memory wise.

Chances are, that beta is delayed anyways. IMO we could discuss this
again at the meeting next week.
I want to invite you for the FWT meeting on 2014-12-09, 21:00 CET to
explain your standpoint.

FWT, Everyone OK with that?

Best, Johannes
Post by Philip Bauer
Dear Framework-team,
I got the message that https://dev.plone.org/ticket/20144 should be moved to 5.1, which is generally ok by me but I can't help wondering why.
The implementation, upgrade-steps and tests are all done. Working with folderish dexterity-types in real projects has been tested by many developers for quite some time. What do you think needs more work or improvements for it to be included in Plone 5? I even migrated the test for plone.app.contentmenu (https://github.com/plone/plone.app.contentmenu/pull/8) to work with AT and DX to make sure nothing changes regarding the UI.
I'm more than willing to put in more time if it means having this in Plone 5.0 and not 5.1 (which will be at least a year form now). I think the point when we move from AT to DX is also the right moment to switch from itemish to folderish.
Also: If the -1 stands: You did not answer the question about the alternative folderish profile.
Philip
--
Starzel.de
Philip Bauer
Adlzreiterstr. 35
80337 MÃŒnchen
Tel: 089 - 189 29 533
Fax: 089 - 189 29 535
www.starzel.de
_______________________________________________
Framework-Team mailing list
https://lists.plone.org/mailman/listinfo/plone-framework-team
--
programmatic web development
di(fh) johannes raggam / thet
python plone zope development
plone framework team member
mail: ***@programmatic.pro
web: http://programmatic.pro
http://bluedynamics.com
Eric Bréhault
2014-12-01 13:02:06 UTC
Permalink
Sure, that's fine with me.

Eric
Post by Johannes Raggam
The decision was made because we don't want to have Plone 5 beta further
delayed and some were opting for a feature freeze. The changes in PLIP
20144 might be small and the positive impact big, but there could be
some negative impact: UI wise (although the default behavior isn't
changed), performance wise (you already proofed, that tests run the same
speed with or without folderish base types) or memory wise.
Chances are, that beta is delayed anyways. IMO we could discuss this
again at the meeting next week.
I want to invite you for the FWT meeting on 2014-12-09, 21:00 CET to
explain your standpoint.
FWT, Everyone OK with that?
Best, Johannes
Post by Philip Bauer
Dear Framework-team,
I got the message that https://dev.plone.org/ticket/20144 should be
moved to 5.1, which is generally ok by me but I can't help wondering why.
Post by Philip Bauer
The implementation, upgrade-steps and tests are all done. Working with
folderish dexterity-types in real projects has been tested by many
developers for quite some time. What do you think needs more work or
improvements for it to be included in Plone 5? I even migrated the test for
plone.app.contentmenu (
https://github.com/plone/plone.app.contentmenu/pull/8) to work with AT
and DX to make sure nothing changes regarding the UI.
Post by Philip Bauer
I'm more than willing to put in more time if it means having this in
Plone 5.0 and not 5.1 (which will be at least a year form now). I think the
point when we move from AT to DX is also the right moment to switch from
itemish to folderish.
Post by Philip Bauer
Also: If the -1 stands: You did not answer the question about the
alternative folderish profile.
Post by Philip Bauer
Philip
--
Starzel.de
Philip Bauer
Adlzreiterstr. 35
80337 MÃŒnchen
Tel: 089 - 189 29 533
Fax: 089 - 189 29 535
www.starzel.de
_______________________________________________
Framework-Team mailing list
https://lists.plone.org/mailman/listinfo/plone-framework-team
--
programmatic web development
di(fh) johannes raggam / thet
python plone zope development
plone framework team member
web: http://programmatic.pro
http://bluedynamics.com
_______________________________________________
Framework-Team mailing list
https://lists.plone.org/mailman/listinfo/plone-framework-team
Jens W. Klein
2014-12-01 13:10:27 UTC
Permalink
Hi,

+1 for inviting Philip to the FWT meeting to discuss this topic.

I agree we have lots of advantages.

What I kept in mind from our discussion is one major risk: Third party
code depending on IFolderish queries will break, because theres no
difference any more.

We need to have a clear way to check if something is a container by
intend or just because its a folderish type (which is not used as
folderish, just prepared to be used this way).

For more see also the FWT protocol posted earlier on this list.

greets Jens
Post by Johannes Raggam
The decision was made because we don't want to have Plone 5 beta further
delayed and some were opting for a feature freeze. The changes in PLIP
20144 might be small and the positive impact big, but there could be
some negative impact: UI wise (although the default behavior isn't
changed), performance wise (you already proofed, that tests run the same
speed with or without folderish base types) or memory wise.
Chances are, that beta is delayed anyways. IMO we could discuss this
again at the meeting next week.
I want to invite you for the FWT meeting on 2014-12-09, 21:00 CET to
explain your standpoint.
FWT, Everyone OK with that?
Best, Johannes
Post by Philip Bauer
Dear Framework-team,
I got the message that https://dev.plone.org/ticket/20144 should be moved to 5.1, which is generally ok by me but I can't help wondering why.
The implementation, upgrade-steps and tests are all done. Working with folderish dexterity-types in real projects has been tested by many developers for quite some time. What do you think needs more work or improvements for it to be included in Plone 5? I even migrated the test for plone.app.contentmenu (https://github.com/plone/plone.app.contentmenu/pull/8) to work with AT and DX to make sure nothing changes regarding the UI.
I'm more than willing to put in more time if it means having this in Plone 5.0 and not 5.1 (which will be at least a year form now). I think the point when we move from AT to DX is also the right moment to switch from itemish to folderish.
Also: If the -1 stands: You did not answer the question about the alternative folderish profile.
Philip
--
Starzel.de
Philip Bauer
Adlzreiterstr. 35
80337 München
Tel: 089 - 189 29 533
Fax: 089 - 189 29 535
www.starzel.de
_______________________________________________
Framework-Team mailing list
https://lists.plone.org/mailman/listinfo/plone-framework-team
--
Klein & Partner KG, member of BlueDynamics Alliance
Timo Stollenwerk
2014-12-01 14:34:04 UTC
Permalink
Dylan wrote (https://dev.plone.org/ticket/20144#comment:12):

"You need to read what I wrote more carefully. I'm not going to explain
them a third time. There are UX bugs with this current proposal that
will need to be addressed to prevent confusion when using sharing,
portlets and navigation titles. There is no point introducing a feature
if it makes the product more complicated. Please refer to recent UI team
discussion on this PLIP."

As I said before on the FWT meeting. I'm not opposed at all to the idea
of having everything folderish. Though, the discussion is not new and I
don't think we are not going to solve a debate that has been around for
year in the last minutes before a major release.

BTW: Did we assign a FWT member for an initial review of the PLIP?

Timo
Post by Jens W. Klein
Hi,
+1 for inviting Philip to the FWT meeting to discuss this topic.
I agree we have lots of advantages.
What I kept in mind from our discussion is one major risk: Third party
code depending on IFolderish queries will break, because theres no
difference any more.
We need to have a clear way to check if something is a container by
intend or just because its a folderish type (which is not used as
folderish, just prepared to be used this way).
For more see also the FWT protocol posted earlier on this list.
greets Jens
Post by Johannes Raggam
The decision was made because we don't want to have Plone 5 beta further
delayed and some were opting for a feature freeze. The changes in PLIP
20144 might be small and the positive impact big, but there could be
some negative impact: UI wise (although the default behavior isn't
changed), performance wise (you already proofed, that tests run the same
speed with or without folderish base types) or memory wise.
Chances are, that beta is delayed anyways. IMO we could discuss this
again at the meeting next week.
I want to invite you for the FWT meeting on 2014-12-09, 21:00 CET to
explain your standpoint.
FWT, Everyone OK with that?
Best, Johannes
Post by Philip Bauer
Dear Framework-team,
I got the message that https://dev.plone.org/ticket/20144 should be
moved to 5.1, which is generally ok by me but I can't help wondering
why.
The implementation, upgrade-steps and tests are all done. Working
with folderish dexterity-types in real projects has been tested by
many developers for quite some time. What do you think needs more
work or improvements for it to be included in Plone 5? I even
migrated the test for plone.app.contentmenu
(https://github.com/plone/plone.app.contentmenu/pull/8) to work with
AT and DX to make sure nothing changes regarding the UI.
I'm more than willing to put in more time if it means having this in
Plone 5.0 and not 5.1 (which will be at least a year form now). I
think the point when we move from AT to DX is also the right moment
to switch from itemish to folderish.
Also: If the -1 stands: You did not answer the question about the
alternative folderish profile.
Philip
--
Starzel.de
Philip Bauer
Adlzreiterstr. 35
80337 München
Tel: 089 - 189 29 533
Fax: 089 - 189 29 535
www.starzel.de
_______________________________________________
Framework-Team mailing list
https://lists.plone.org/mailman/listinfo/plone-framework-team
Timo Stollenwerk
2014-12-01 14:23:06 UTC
Permalink
Post by Johannes Raggam
The decision was made because we don't want to have Plone 5 beta further
delayed and some were opting for a feature freeze.
As far as I know we already declared a feature freeze quite some time
ago. Eric confirmed that during the last FWT meeting, right?
Post by Johannes Raggam
The changes in PLIP
20144 might be small and the positive impact big, but there could be
some negative impact: UI wise (although the default behavior isn't
changed),
Which is also one major point why I'm -1. We are making a small change,
that is inconsistent (some objects will be folderish, others are not),
just to somehow get closer to the "real" goal (all objects folderish,
also fundamental UI changes).

Dylan investigated the possibilities to move the 100% to folderish types
option and the last thing that I heard was that there are some serious
UI problems that can not be solved easily. Before we move into that
direction, we should think carefully about where we are heading before
we start with the implementation and moving things into the core.

Also we are going to confuse developers by introducing another way of
doing things and changing one of basic principles of Plone development
(that a folderish object is a container that implements IFolderish). If
we are serious about making Plone development easier, we have to remove
inconsistencies, not introducing more of them.

In addition, we already have at least three versions of p.a.contenttypes
that we need to support. Do you really want to add and maintain a fourth
and fifth version in parallel?
Post by Johannes Raggam
performance wise (you already proofed, that tests run the same
speed with or without folderish base types) or memory wise.
Measuring test execution time does not prove at all that there are no
performance impacts.
Post by Johannes Raggam
Chances are, that beta is delayed anyways. IMO we could discuss this
again at the meeting next week.
I want to invite you for the FWT meeting on 2014-12-09, 21:00 CET to
explain your standpoint.
FWT, Everyone OK with that?
I don't see how that would change our decision from the last meeting. I
would really like us to focus on getting Plone 5 out of the door (going
through the blockers, discussing how we can speed up things, etc.).

Timo
Johannes Raggam
2014-12-02 09:57:48 UTC
Permalink
Post by Philip Bauer
Dear Framework-team,
I got the message that https://dev.plone.org/ticket/20144 should be moved to 5.1, which is generally ok by me but I can't help wondering why.
The implementation, upgrade-steps and tests are all done. Working with folderish dexterity-types in real projects has been tested by many developers for quite some time. What do you think needs more work or improvements for it to be included in Plone 5? I even migrated the test for plone.app.contentmenu (https://github.com/plone/plone.app.contentmenu/pull/8) to work with AT and DX to make sure nothing changes regarding the UI.
I'm more than willing to put in more time if it means having this in Plone 5.0 and not 5.1 (which will be at least a year form now). I think the point when we move from AT to DX is also the right moment to switch from itemish to folderish.
Also: If the -1 stands: You did not answer the question about the alternative folderish profile.
I'm +1 to keep this "folderish" profile. This way, we can start using it
immediately without depending on external addons like
collective.folderishtypes. People will use it and we can get feedback
for folderishtypes in Plone.

When we folderish types by default, we should deprecate this profile.
--
programmatic web development
di(fh) johannes raggam / thet
python plone zope development
plone framework team member
mail: ***@programmatic.pro
web: http://programmatic.pro
http://bluedynamics.com
Timo Stollenwerk
2014-12-02 10:01:45 UTC
Permalink
Post by Johannes Raggam
Post by Philip Bauer
Dear Framework-team,
I got the message that https://dev.plone.org/ticket/20144 should be moved to 5.1, which is generally ok by me but I can't help wondering why.
The implementation, upgrade-steps and tests are all done. Working with folderish dexterity-types in real projects has been tested by many developers for quite some time. What do you think needs more work or improvements for it to be included in Plone 5? I even migrated the test for plone.app.contentmenu (https://github.com/plone/plone.app.contentmenu/pull/8) to work with AT and DX to make sure nothing changes regarding the UI.
I'm more than willing to put in more time if it means having this in Plone 5.0 and not 5.1 (which will be at least a year form now). I think the point when we move from AT to DX is also the right moment to switch from itemish to folderish.
Also: If the -1 stands: You did not answer the question about the alternative folderish profile.
I'm +1 to keep this "folderish" profile. This way, we can start using it
immediately without depending on external addons like
collective.folderishtypes. People will use it and we can get feedback
for folderishtypes in Plone.
I'm -1 on this. It is confusing and I don't see a good reason why this
can not be accomplished in an add-on product. For details, see my notes
on the pull request:

https://github.com/plone/plone.app.contenttypes/pull/187

Timo
Johannes Raggam
2014-12-09 09:22:48 UTC
Permalink
hi,

just to be prepared: are you joining our FWT meeting today at 21:00 CET?

johannes
Post by Philip Bauer
Dear Framework-team,
I got the message that https://dev.plone.org/ticket/20144 should be moved to 5.1, which is generally ok by me but I can't help wondering why.
The implementation, upgrade-steps and tests are all done. Working with folderish dexterity-types in real projects has been tested by many developers for quite some time. What do you think needs more work or improvements for it to be included in Plone 5? I even migrated the test for plone.app.contentmenu (https://github.com/plone/plone.app.contentmenu/pull/8) to work with AT and DX to make sure nothing changes regarding the UI.
I'm more than willing to put in more time if it means having this in Plone 5.0 and not 5.1 (which will be at least a year form now). I think the point when we move from AT to DX is also the right moment to switch from itemish to folderish.
Also: If the -1 stands: You did not answer the question about the alternative folderish profile.
Philip
--
Starzel.de
Philip Bauer
Adlzreiterstr. 35
80337 MÃŒnchen
Tel: 089 - 189 29 533
Fax: 089 - 189 29 535
www.starzel.de
_______________________________________________
Framework-Team mailing list
https://lists.plone.org/mailman/listinfo/plone-framework-team
Philip Bauer
2014-12-09 10:16:06 UTC
Permalink
Sorry, I can't join you beards tonight because I give a Plone Coding Dojo at the python-usergroup :)


I'm absolutely ok with waiting until 5.1 to go fully-folderish if you think it is to risky now. A feature-freeze is also be a really good reason not to add any new features.

Anyway I added information on speed and weight to https://dev.plone.org/ticket/20144. The short version: No problem.

If we don't go there now I'm still much in favor of keeping a optional folderish profile in the package (instead of pointing developers to collective.folderishtypes) because this way I can add a easy upgrade-step that migrates the from the folderish profile types to the default-types in p.a.c. once they are folderish without having to uninstall a package.

The IFolderish-query problem that Jens raised might be a real problem. Although querying for IFolderish is actually wrong, instead you should query for is_folderish which respects INonStructuralFolder. Adding INonStructuralFolder to items depending on filter_content_types and allowed_content_types might be possible. We'd also have to reindex is_folderish on changing these two settings.

UX-Problems 'only' arise when people stop using folders and instead only use documents. Dylan has a point there:
1. You can no longer have Portlets only on the "frontpage" of a folder. Dylan is right in that we need a feature in portlets that allows portlets to be only registered to an item but not it's children.

2. Same goes for sharing: You can only delegate permissions for the objects and its children, not only the object

3. Default-pages: Folderish types that you can add content to cannot be made into default pages. This can for only be overridden by adding types to the setting default_page_types in site_properties.

@Timo: Why do we have three version of p.a.c that we support? As far as I can see we only have one. Do you mean ATContentTypes?


If I may be so bold to annoy you with my take on three more subjects:

1. Tests: I realized that most test do not use the dexterity types at all. It might be that we miss some problems because of that. I changed the tests for plone.app.contentmenu to test both AT and DX (https://github.com/plone/plone.app.contentmenu/pull/8) but the test-setup is a little cumbersome. We need a canonical test-setup to test both frameworks otherwise all packages do it differently and future developers will a good reason to hate us.

2. Did you talk about deprecating AT-types? Editing AT-types is semi-broken in the current coredev (I guess only a js-problems). I am also working on small changes to ATContentTypes that will allow add-ons (PloneFormGen!) that depend on parts of ATContentTypes to work without uninstalling the types from p.a.c by depending on a new profile "base": https://github.com/starzel/Products.ATContentTypes/commit/1c4aff0c57ff8ea2b0001b44d78f08f9083b0a8f

3. https://pypi.python.org/pypi/bobtemplates.plone might become the new canonical tool for creating new eggs and I'd love to see it included in the Plone 5 installers. Would the framework-team have an issue with that?


Have a great meeting!
Philip

--
Starzel.de
Philip Bauer
Adlzreiterstr. 35
80337 München
Tel: 089 - 189 29 533
Fax: 089 - 189 29 535
***@starzel.de
www.starzel.de
Post by Johannes Raggam
hi,
just to be prepared: are you joining our FWT meeting today at 21:00 CET?
johannes
Post by Philip Bauer
Dear Framework-team,
I got the message that https://dev.plone.org/ticket/20144 should be moved to 5.1, which is generally ok by me but I can't help wondering why.
The implementation, upgrade-steps and tests are all done. Working with folderish dexterity-types in real projects has been tested by many developers for quite some time. What do you think needs more work or improvements for it to be included in Plone 5? I even migrated the test for plone.app.contentmenu (https://github.com/plone/plone.app.contentmenu/pull/8) to work with AT and DX to make sure nothing changes regarding the UI.
I'm more than willing to put in more time if it means having this in Plone 5.0 and not 5.1 (which will be at least a year form now). I think the point when we move from AT to DX is also the right moment to switch from itemish to folderish.
Also: If the -1 stands: You did not answer the question about the alternative folderish profile.
Philip
--
Starzel.de
Philip Bauer
Adlzreiterstr. 35
80337 München
Tel: 089 - 189 29 533
Fax: 089 - 189 29 535
www.starzel.de
_______________________________________________
Framework-Team mailing list
https://lists.plone.org/mailman/listinfo/plone-framework-team
Timo Stollenwerk
2014-12-09 10:41:21 UTC
Permalink
Post by Philip Bauer
Sorry, I can't join you beards tonight because I give a Plone Coding Dojo at the python-usergroup :)
I'm absolutely ok with waiting until 5.1 to go fully-folderish if you think it is to risky now. A feature-freeze is also be a really good reason not to add any new features.
Anyway I added information on speed and weight to https://dev.plone.org/ticket/20144. The short version: No problem.
If we don't go there now I'm still much in favor of keeping a optional folderish profile in the package (instead of pointing developers to collective.folderishtypes) because this way I can add a easy upgrade-step that migrates the from the folderish profile types to the default-types in p.a.c. once they are folderish without having to uninstall a package.
The IFolderish-query problem that Jens raised might be a real problem. Although querying for IFolderish is actually wrong, instead you should query for is_folderish which respects INonStructuralFolder. Adding INonStructuralFolder to items depending on filter_content_types and allowed_content_types might be possible. We'd also have to reindex is_folderish on changing these two settings.
1. You can no longer have Portlets only on the "frontpage" of a folder. Dylan is right in that we need a feature in portlets that allows portlets to be only registered to an item but not it's children.
2. Same goes for sharing: You can only delegate permissions for the objects and its children, not only the object
3. Default-pages: Folderish types that you can add content to cannot be made into default pages. This can for only be overridden by adding types to the setting default_page_types in site_properties.
@Timo: Why do we have three version of p.a.c that we support? As far as I can see we only have one. Do you mean ATContentTypes?
plone.app.contenttypes branches: 1.0.x / 1.1.x / 1.2.x

A folderish branch (1.3.x) would be the fourth version. I don't think we
can just focus on the latest branch and ignore the rest.

Personally I still use the 1.0.x branch. Both 1.1.x and 1.2.x are still
in beta and I did not dare to use them in production yet (I tried 1.1.x
some time ago but gave up, mostly because I did not need any of the new
features).

Timo
Timo Stollenwerk
2014-12-09 11:10:37 UTC
Permalink
Post by Philip Bauer
1. Tests: I realized that most test do not use the dexterity types at all. It might be that we miss some problems because of that. I changed the tests for plone.app.contentmenu to test both AT and DX (https://github.com/plone/plone.app.contentmenu/pull/8) but the test-setup is a little cumbersome. We need a canonical test-setup to test both frameworks otherwise all packages do it differently and future developers will a good reason to hate us.
Thanks for bringing this up! :)

The initial idea was to move all tests to use Dexterity types or the
p.a.contenttypes fixture (if necessary). I don't think a generic test
fixture that supports both AT and DX is possible.

Tests with tons of if/else statements for DX/AT are a horrible idea in
my optinion. They are a nightmare to maintain, to debug and really hard
to understand.

Therefore, even though it is not perfect, I think we should stick with
the approach to test against Dexterity only. For some packages it might
be necessary to actually test both, but those should be the exception
from the rule. Most packages just need "some" content object and it does
not really matter if it is a DX or AT object.

One problem I see with that approach is that the p.a.contenttypes
fixture, that most packages use, might be a bit too heavy. If
p.a.contenttypes becomes the de-factor test layer we have to make sure
that we keep it lightweight (because it is incredibly hard to change a
test fixture that is used by so many packages later and it is generally
a bad practice to put stuff into the main fixture if they are only
needed for a couple of tests).

Therefore we should have a close look at the p.a.contenttypes and
p.a.event test fixture and make sure they are as lightweight as possible.

I agree that we need to write down some guidelines for tests and that we
have to work on that. Though, my priorities right now are the control
panels, because they are blocking the Plone 5 beta release. If anybody
wants to work on that I'm happy to help though!

Timo
Johannes Raggam
2014-12-09 12:39:31 UTC
Permalink
Post by Timo Stollenwerk
Post by Philip Bauer
1. Tests: I realized that most test do not use the dexterity types at all. It might be that we miss some problems because of that. I changed the tests for plone.app.contentmenu to test both AT and DX (https://github.com/plone/plone.app.contentmenu/pull/8) but the test-setup is a little cumbersome. We need a canonical test-setup to test both frameworks otherwise all packages do it differently and future developers will a good reason to hate us.
Thanks for bringing this up! :)
yep.
Post by Timo Stollenwerk
The initial idea was to move all tests to use Dexterity types or the
p.a.contenttypes fixture (if necessary). I don't think a generic test
fixture that supports both AT and DX is possible.
Tests with tons of if/else statements for DX/AT are a horrible idea in
my optinion. They are a nightmare to maintain, to debug and really hard
to understand.
In plone.app.event until 1.2 there is a generic test suite for
Archetypes and Dexterity. I think, this approach was quite elegant:

In
https://github.com/plone/plone.app.event/blob/1.2.x/plone/app/event/tests/base_setup.py#L32 there is an ``AbstractSampleDataEvents``, which creates a basic set of content to test with. The abstract ``event_factory`` method was overridden by AT or DX based unit test classes, like here:

https://github.com/plone/plone.app.event/blob/1.2.x/plone/app/event/tests/test_event_view.py#L13

You see, ``TestEventViewDX`` defines all tests cases and the concrete
implementation of ``event_factory`` and uses an DX test layer.
``TestEventViewAT`` below just subclasses the DX one, overrides
``event_factory`` and uses an AT layer:
https://github.com/plone/plone.app.event/blob/1.2.x/plone/app/event/tests/test_event_view.py#L63

So both unit tests classes run the same the test cases.

That's possible, because there are AT and DX content type
accessors/wrappers, which unify ``set`` and ``get`` access to those
types.

Initially this was a proof of concept on how to support both Archetypes
and Dexterity in one package without duplicating code. It works quite
well! But we were removing the Archetypes based code from
plone.app.event 2.0, since this release is Dexterity only.

To the question on what to do with our Plone tests: Instead of creating
accessor methods for AT and DX content types, I think it's better to
rewrite for Dexterity only and move the Archetypes test classes into the
ATContentTypes package before.

Johannes
Post by Timo Stollenwerk
Therefore, even though it is not perfect, I think we should stick with
the approach to test against Dexterity only. For some packages it might
be necessary to actually test both, but those should be the exception
from the rule. Most packages just need "some" content object and it does
not really matter if it is a DX or AT object.
One problem I see with that approach is that the p.a.contenttypes
fixture, that most packages use, might be a bit too heavy. If
p.a.contenttypes becomes the de-factor test layer we have to make sure
that we keep it lightweight (because it is incredibly hard to change a
test fixture that is used by so many packages later and it is generally
a bad practice to put stuff into the main fixture if they are only
needed for a couple of tests).
Therefore we should have a close look at the p.a.contenttypes and
p.a.event test fixture and make sure they are as lightweight as possible.
I agree that we need to write down some guidelines for tests and that we
have to work on that. Though, my priorities right now are the control
panels, because they are blocking the Plone 5 beta release. If anybody
wants to work on that I'm happy to help though!
Timo
Timo Stollenwerk
2014-12-11 10:51:52 UTC
Permalink
Hi,

I just talked to Philip (as discussed at the last FWT meeting). He will
revert the changes and make a pull request. We should invite him (and
maybe also Dylan) to the next FWT meeting to discuss the PLIP in detail.

Cheers,
Timo
Post by Johannes Raggam
Post by Timo Stollenwerk
Post by Philip Bauer
1. Tests: I realized that most test do not use the dexterity types at all. It might be that we miss some problems because of that. I changed the tests for plone.app.contentmenu to test both AT and DX (https://github.com/plone/plone.app.contentmenu/pull/8) but the test-setup is a little cumbersome. We need a canonical test-setup to test both frameworks otherwise all packages do it differently and future developers will a good reason to hate us.
Thanks for bringing this up! :)
yep.
Post by Timo Stollenwerk
The initial idea was to move all tests to use Dexterity types or the
p.a.contenttypes fixture (if necessary). I don't think a generic test
fixture that supports both AT and DX is possible.
Tests with tons of if/else statements for DX/AT are a horrible idea in
my optinion. They are a nightmare to maintain, to debug and really hard
to understand.
In plone.app.event until 1.2 there is a generic test suite for
In
https://github.com/plone/plone.app.event/blob/1.2.x/plone/app/event/tests/test_event_view.py#L13
You see, ``TestEventViewDX`` defines all tests cases and the concrete
implementation of ``event_factory`` and uses an DX test layer.
``TestEventViewAT`` below just subclasses the DX one, overrides
https://github.com/plone/plone.app.event/blob/1.2.x/plone/app/event/tests/test_event_view.py#L63
So both unit tests classes run the same the test cases.
That's possible, because there are AT and DX content type
accessors/wrappers, which unify ``set`` and ``get`` access to those
types.
Initially this was a proof of concept on how to support both Archetypes
and Dexterity in one package without duplicating code. It works quite
well! But we were removing the Archetypes based code from
plone.app.event 2.0, since this release is Dexterity only.
To the question on what to do with our Plone tests: Instead of creating
accessor methods for AT and DX content types, I think it's better to
rewrite for Dexterity only and move the Archetypes test classes into the
ATContentTypes package before.
Johannes
Post by Timo Stollenwerk
Therefore, even though it is not perfect, I think we should stick with
the approach to test against Dexterity only. For some packages it might
be necessary to actually test both, but those should be the exception
from the rule. Most packages just need "some" content object and it does
not really matter if it is a DX or AT object.
One problem I see with that approach is that the p.a.contenttypes
fixture, that most packages use, might be a bit too heavy. If
p.a.contenttypes becomes the de-factor test layer we have to make sure
that we keep it lightweight (because it is incredibly hard to change a
test fixture that is used by so many packages later and it is generally
a bad practice to put stuff into the main fixture if they are only
needed for a couple of tests).
Therefore we should have a close look at the p.a.contenttypes and
p.a.event test fixture and make sure they are as lightweight as possible.
I agree that we need to write down some guidelines for tests and that we
have to work on that. Though, my priorities right now are the control
panels, because they are blocking the Plone 5 beta release. If anybody
wants to work on that I'm happy to help though!
Timo
Roel Bruggink
2014-12-11 11:49:11 UTC
Permalink
Please keep all discussions on the mailinglist or PLIP ticket when
possible, so the process in transparent to all community members.

Cheers!
Post by Timo Stollenwerk
Hi,
I just talked to Philip (as discussed at the last FWT meeting). He will
revert the changes and make a pull request. We should invite him (and
maybe also Dylan) to the next FWT meeting to discuss the PLIP in detail.
Cheers,
Timo
Post by Johannes Raggam
Post by Timo Stollenwerk
Post by Philip Bauer
1. Tests: I realized that most test do not use the dexterity types at
all. It might be that we miss some problems because of that. I changed the
tests for plone.app.contentmenu to test both AT and DX (
https://github.com/plone/plone.app.contentmenu/pull/8) but the test-setup
is a little cumbersome. We need a canonical test-setup to test both
frameworks otherwise all packages do it differently and future developers
will a good reason to hate us.
Post by Johannes Raggam
Post by Timo Stollenwerk
Thanks for bringing this up! :)
yep.
Post by Timo Stollenwerk
The initial idea was to move all tests to use Dexterity types or the
p.a.contenttypes fixture (if necessary). I don't think a generic test
fixture that supports both AT and DX is possible.
Tests with tons of if/else statements for DX/AT are a horrible idea in
my optinion. They are a nightmare to maintain, to debug and really hard
to understand.
In plone.app.event until 1.2 there is a generic test suite for
In
https://github.com/plone/plone.app.event/blob/1.2.x/plone/app/event/tests/base_setup.py#L32
there is an ``AbstractSampleDataEvents``, which creates a basic set of
content to test with. The abstract ``event_factory`` method was overridden
https://github.com/plone/plone.app.event/blob/1.2.x/plone/app/event/tests/test_event_view.py#L13
Post by Johannes Raggam
You see, ``TestEventViewDX`` defines all tests cases and the concrete
implementation of ``event_factory`` and uses an DX test layer.
``TestEventViewAT`` below just subclasses the DX one, overrides
https://github.com/plone/plone.app.event/blob/1.2.x/plone/app/event/tests/test_event_view.py#L63
Post by Johannes Raggam
So both unit tests classes run the same the test cases.
That's possible, because there are AT and DX content type
accessors/wrappers, which unify ``set`` and ``get`` access to those
types.
Initially this was a proof of concept on how to support both Archetypes
and Dexterity in one package without duplicating code. It works quite
well! But we were removing the Archetypes based code from
plone.app.event 2.0, since this release is Dexterity only.
To the question on what to do with our Plone tests: Instead of creating
accessor methods for AT and DX content types, I think it's better to
rewrite for Dexterity only and move the Archetypes test classes into the
ATContentTypes package before.
Johannes
Post by Timo Stollenwerk
Therefore, even though it is not perfect, I think we should stick with
the approach to test against Dexterity only. For some packages it might
be necessary to actually test both, but those should be the exception
from the rule. Most packages just need "some" content object and it does
not really matter if it is a DX or AT object.
One problem I see with that approach is that the p.a.contenttypes
fixture, that most packages use, might be a bit too heavy. If
p.a.contenttypes becomes the de-factor test layer we have to make sure
that we keep it lightweight (because it is incredibly hard to change a
test fixture that is used by so many packages later and it is generally
a bad practice to put stuff into the main fixture if they are only
needed for a couple of tests).
Therefore we should have a close look at the p.a.contenttypes and
p.a.event test fixture and make sure they are as lightweight as
possible.
Post by Johannes Raggam
Post by Timo Stollenwerk
I agree that we need to write down some guidelines for tests and that we
have to work on that. Though, my priorities right now are the control
panels, because they are blocking the Plone 5 beta release. If anybody
wants to work on that I'm happy to help though!
Timo
_______________________________________________
Framework-Team mailing list
https://lists.plone.org/mailman/listinfo/plone-framework-team
--
Rob Gietema
https://www.fourdigits.nl/over-ons#roel-bruggink

Four Digits BV
https://www.fourdigits.nl tel: +31 26 4422700
Philip Bauer
2014-12-11 13:21:40 UTC
Permalink
I added my notes and information on the changes I did today to https://dev.plone.org/ticket/20144

There are now pull-requests for this PLIP (https://github.com/plone/plone.app.contenttypes/pull/196) and for the folderish profile (https://github.com/plone/plone.app.contenttypes/pull/195).

I'll join you on the 23rd. Maybe we can find solutions for the issues Dylan raised.

Philip

--
Starzel.de
Philip Bauer
Adlzreiterstr. 35
80337 München
Tel: 089 - 189 29 533
Fax: 089 - 189 29 535
***@starzel.de
www.starzel.de
Please keep all discussions on the mailinglist or PLIP ticket when possible, so the process in transparent to all community members.
Cheers!
Hi,
I just talked to Philip (as discussed at the last FWT meeting). He will
revert the changes and make a pull request. We should invite him (and
maybe also Dylan) to the next FWT meeting to discuss the PLIP in detail.
Cheers,
Timo
Post by Johannes Raggam
Post by Timo Stollenwerk
Post by Philip Bauer
1. Tests: I realized that most test do not use the dexterity types at all. It might be that we miss some problems because of that. I changed the tests for plone.app.contentmenu to test both AT and DX (https://github.com/plone/plone.app.contentmenu/pull/8) but the test-setup is a little cumbersome. We need a canonical test-setup to test both frameworks otherwise all packages do it differently and future developers will a good reason to hate us.
Thanks for bringing this up! :)
yep.
Post by Timo Stollenwerk
The initial idea was to move all tests to use Dexterity types or the
p.a.contenttypes fixture (if necessary). I don't think a generic test
fixture that supports both AT and DX is possible.
Tests with tons of if/else statements for DX/AT are a horrible idea in
my optinion. They are a nightmare to maintain, to debug and really hard
to understand.
In plone.app.event until 1.2 there is a generic test suite for
In
https://github.com/plone/plone.app.event/blob/1.2.x/plone/app/event/tests/test_event_view.py#L13
You see, ``TestEventViewDX`` defines all tests cases and the concrete
implementation of ``event_factory`` and uses an DX test layer.
``TestEventViewAT`` below just subclasses the DX one, overrides
https://github.com/plone/plone.app.event/blob/1.2.x/plone/app/event/tests/test_event_view.py#L63
So both unit tests classes run the same the test cases.
That's possible, because there are AT and DX content type
accessors/wrappers, which unify ``set`` and ``get`` access to those
types.
Initially this was a proof of concept on how to support both Archetypes
and Dexterity in one package without duplicating code. It works quite
well! But we were removing the Archetypes based code from
plone.app.event 2.0, since this release is Dexterity only.
To the question on what to do with our Plone tests: Instead of creating
accessor methods for AT and DX content types, I think it's better to
rewrite for Dexterity only and move the Archetypes test classes into the
ATContentTypes package before.
Johannes
Post by Timo Stollenwerk
Therefore, even though it is not perfect, I think we should stick with
the approach to test against Dexterity only. For some packages it might
be necessary to actually test both, but those should be the exception
from the rule. Most packages just need "some" content object and it does
not really matter if it is a DX or AT object.
One problem I see with that approach is that the p.a.contenttypes
fixture, that most packages use, might be a bit too heavy. If
p.a.contenttypes becomes the de-factor test layer we have to make sure
that we keep it lightweight (because it is incredibly hard to change a
test fixture that is used by so many packages later and it is generally
a bad practice to put stuff into the main fixture if they are only
needed for a couple of tests).
Therefore we should have a close look at the p.a.contenttypes and
p.a.event test fixture and make sure they are as lightweight as possible.
I agree that we need to write down some guidelines for tests and that we
have to work on that. Though, my priorities right now are the control
panels, because they are blocking the Plone 5 beta release. If anybody
wants to work on that I'm happy to help though!
Timo
_______________________________________________
Framework-Team mailing list
https://lists.plone.org/mailman/listinfo/plone-framework-team
--
Rob Gietema
https://www.fourdigits.nl/over-ons#roel-bruggink
Four Digits BV
https://www.fourdigits.nl tel: +31 26 4422700
_______________________________________________
Framework-Team mailing list
https://lists.plone.org/mailman/listinfo/plone-framework-team
Philip Bauer
2014-12-11 13:21:41 UTC
Permalink
I added my notes and information on the changes I did today to https://dev.plone.org/ticket/20144

There are now pull-requests for this PLIP (https://github.com/plone/plone.app.contenttypes/pull/196) and for the folderish profile (https://github.com/plone/plone.app.contenttypes/pull/195).

I'll join you on the 23rd. Maybe we can find solutions for the issues Dylan raised.

Philip

--
Starzel.de
Philip Bauer
Adlzreiterstr. 35
80337 München
Tel: 089 - 189 29 533
Fax: 089 - 189 29 535
***@starzel.de
www.starzel.de
Please keep all discussions on the mailinglist or PLIP ticket when possible, so the process in transparent to all community members.
Cheers!
Hi,
I just talked to Philip (as discussed at the last FWT meeting). He will
revert the changes and make a pull request. We should invite him (and
maybe also Dylan) to the next FWT meeting to discuss the PLIP in detail.
Cheers,
Timo
Post by Johannes Raggam
Post by Timo Stollenwerk
Post by Philip Bauer
1. Tests: I realized that most test do not use the dexterity types at all. It might be that we miss some problems because of that. I changed the tests for plone.app.contentmenu to test both AT and DX (https://github.com/plone/plone.app.contentmenu/pull/8) but the test-setup is a little cumbersome. We need a canonical test-setup to test both frameworks otherwise all packages do it differently and future developers will a good reason to hate us.
Thanks for bringing this up! :)
yep.
Post by Timo Stollenwerk
The initial idea was to move all tests to use Dexterity types or the
p.a.contenttypes fixture (if necessary). I don't think a generic test
fixture that supports both AT and DX is possible.
Tests with tons of if/else statements for DX/AT are a horrible idea in
my optinion. They are a nightmare to maintain, to debug and really hard
to understand.
In plone.app.event until 1.2 there is a generic test suite for
In
https://github.com/plone/plone.app.event/blob/1.2.x/plone/app/event/tests/test_event_view.py#L13
You see, ``TestEventViewDX`` defines all tests cases and the concrete
implementation of ``event_factory`` and uses an DX test layer.
``TestEventViewAT`` below just subclasses the DX one, overrides
https://github.com/plone/plone.app.event/blob/1.2.x/plone/app/event/tests/test_event_view.py#L63
So both unit tests classes run the same the test cases.
That's possible, because there are AT and DX content type
accessors/wrappers, which unify ``set`` and ``get`` access to those
types.
Initially this was a proof of concept on how to support both Archetypes
and Dexterity in one package without duplicating code. It works quite
well! But we were removing the Archetypes based code from
plone.app.event 2.0, since this release is Dexterity only.
To the question on what to do with our Plone tests: Instead of creating
accessor methods for AT and DX content types, I think it's better to
rewrite for Dexterity only and move the Archetypes test classes into the
ATContentTypes package before.
Johannes
Post by Timo Stollenwerk
Therefore, even though it is not perfect, I think we should stick with
the approach to test against Dexterity only. For some packages it might
be necessary to actually test both, but those should be the exception
from the rule. Most packages just need "some" content object and it does
not really matter if it is a DX or AT object.
One problem I see with that approach is that the p.a.contenttypes
fixture, that most packages use, might be a bit too heavy. If
p.a.contenttypes becomes the de-factor test layer we have to make sure
that we keep it lightweight (because it is incredibly hard to change a
test fixture that is used by so many packages later and it is generally
a bad practice to put stuff into the main fixture if they are only
needed for a couple of tests).
Therefore we should have a close look at the p.a.contenttypes and
p.a.event test fixture and make sure they are as lightweight as possible.
I agree that we need to write down some guidelines for tests and that we
have to work on that. Though, my priorities right now are the control
panels, because they are blocking the Plone 5 beta release. If anybody
wants to work on that I'm happy to help though!
Timo
_______________________________________________
Framework-Team mailing list
https://lists.plone.org/mailman/listinfo/plone-framework-team
--
Rob Gietema
https://www.fourdigits.nl/over-ons#roel-bruggink
Four Digits BV
https://www.fourdigits.nl tel: +31 26 4422700
_______________________________________________
Framework-Team mailing list
https://lists.plone.org/mailman/listinfo/plone-framework-team
Timo Stollenwerk
2014-12-11 13:54:21 UTC
Permalink
The discussion has been documented here:

https://docs.google.com/a/plone.org/document/d/1_SCJYQWu2TmMExNQSV8rI7F5oU2iNA6YkpY0vhX-7SA/edit#

Nothing new since the last meeting(s).

Timo
Post by Roel Bruggink
Please keep all discussions on the mailinglist or PLIP ticket when
possible, so the process in transparent to all community members.
Cheers!
Hi,
I just talked to Philip (as discussed at the last FWT meeting). He will
revert the changes and make a pull request. We should invite him (and
maybe also Dylan) to the next FWT meeting to discuss the PLIP in detail.
Cheers,
Timo
Post by Johannes Raggam
Post by Timo Stollenwerk
Post by Philip Bauer
1. Tests: I realized that most test do not use the dexterity
types at all. It might be that we miss some problems because of
that. I changed the tests for plone.app.contentmenu to test both AT
and DX (https://github.com/plone/plone.app.contentmenu/pull/8) but
the test-setup is a little cumbersome. We need a canonical
test-setup to test both frameworks otherwise all packages do it
differently and future developers will a good reason to hate us.
Post by Johannes Raggam
Post by Timo Stollenwerk
Thanks for bringing this up! :)
yep.
Post by Timo Stollenwerk
The initial idea was to move all tests to use Dexterity types or the
p.a.contenttypes fixture (if necessary). I don't think a generic test
fixture that supports both AT and DX is possible.
Tests with tons of if/else statements for DX/AT are a horrible
idea in
Post by Johannes Raggam
Post by Timo Stollenwerk
my optinion. They are a nightmare to maintain, to debug and
really hard
Post by Johannes Raggam
Post by Timo Stollenwerk
to understand.
In plone.app.event until 1.2 there is a generic test suite for
In
https://github.com/plone/plone.app.event/blob/1.2.x/plone/app/event/tests/base_setup.py#L32
there is an ``AbstractSampleDataEvents``, which creates a basic set
of content to test with. The abstract ``event_factory`` method was
https://github.com/plone/plone.app.event/blob/1.2.x/plone/app/event/tests/test_event_view.py#L13
Post by Johannes Raggam
You see, ``TestEventViewDX`` defines all tests cases and the concrete
implementation of ``event_factory`` and uses an DX test layer.
``TestEventViewAT`` below just subclasses the DX one, overrides
https://github.com/plone/plone.app.event/blob/1.2.x/plone/app/event/tests/test_event_view.py#L63
Post by Johannes Raggam
So both unit tests classes run the same the test cases.
That's possible, because there are AT and DX content type
accessors/wrappers, which unify ``set`` and ``get`` access to those
types.
Initially this was a proof of concept on how to support both
Archetypes
Post by Johannes Raggam
and Dexterity in one package without duplicating code. It works quite
well! But we were removing the Archetypes based code from
plone.app.event 2.0, since this release is Dexterity only.
To the question on what to do with our Plone tests: Instead of
creating
Post by Johannes Raggam
accessor methods for AT and DX content types, I think it's better to
rewrite for Dexterity only and move the Archetypes test classes
into the
Post by Johannes Raggam
ATContentTypes package before.
Johannes
Post by Timo Stollenwerk
Therefore, even though it is not perfect, I think we should stick
with
Post by Johannes Raggam
Post by Timo Stollenwerk
the approach to test against Dexterity only. For some packages it
might
Post by Johannes Raggam
Post by Timo Stollenwerk
be necessary to actually test both, but those should be the exception
from the rule. Most packages just need "some" content object and
it does
Post by Johannes Raggam
Post by Timo Stollenwerk
not really matter if it is a DX or AT object.
One problem I see with that approach is that the p.a.contenttypes
fixture, that most packages use, might be a bit too heavy. If
p.a.contenttypes becomes the de-factor test layer we have to make
sure
Post by Johannes Raggam
Post by Timo Stollenwerk
that we keep it lightweight (because it is incredibly hard to
change a
Post by Johannes Raggam
Post by Timo Stollenwerk
test fixture that is used by so many packages later and it is
generally
Post by Johannes Raggam
Post by Timo Stollenwerk
a bad practice to put stuff into the main fixture if they are only
needed for a couple of tests).
Therefore we should have a close look at the p.a.contenttypes and
p.a.event test fixture and make sure they are as lightweight as
possible.
Post by Johannes Raggam
Post by Timo Stollenwerk
I agree that we need to write down some guidelines for tests and
that we
Post by Johannes Raggam
Post by Timo Stollenwerk
have to work on that. Though, my priorities right now are the control
panels, because they are blocking the Plone 5 beta release. If
anybody
Post by Johannes Raggam
Post by Timo Stollenwerk
wants to work on that I'm happy to help though!
Timo
_______________________________________________
Framework-Team mailing list
https://lists.plone.org/mailman/listinfo/plone-framework-team
--
Rob Gietema
https://www.fourdigits.nl/over-ons#roel-bruggink
<https://www.fourdigits.nl/over-ons#roel-bruggink>
Four Digits BV
https://www.fourdigits.nl <https://www.fourdigits.nl/> tel: +31 26 4422700
Loading...