4teamwork / ftw.builder

Builder pattern for creating Plone objects in tests
3 stars 0 forks source link

Possibly restore Plone 4.1 compatibility #34

Closed lukasgraf closed 9 years ago

lukasgraf commented 9 years ago

Currently ftw.builder is still used by some packages that offer Plone 4.1 compatibility (ftw.contentpage and ftw.contenttemplates) to name just two).

Because the recent change from #32 now unconditionally imports the Dexterity builders, ftw.builder will fail on Plone 4.1 because Dexterity isn't shipped by default up until Plone 4.2.

We have a couple options here:

@jone @maethu @phgross @deiferni

What do you think?

jone commented 9 years ago

Oh, didn't see that coming. We have no 4.1 test (and thus no official support) because we test Dexterity builders with p.a.contenttypes, which is hard to install for 4.1.

I would just move the imports DexterityBuilder (and ArchetypesBuilder) in content.py into the if / else, so that other packages can use the builder with Plone 4.1 again (although still not officially supported).

We could add 4.1 tests, but I think we'd either have to ignore all dexterity tests or at least stop using p.a.contenttypes in tests..

lukasgraf commented 9 years ago

Yeah, I think there's two separate issues here: Not breaking support for using ftw.builder with 4.1 vs. actually testing ftw.builder with 4.1.

I'd also tend to go with the pragmatic approach and just make the imports conditional wherever it's needed.

lukasgraf commented 9 years ago

@jone this should do the trick.

lukasgraf commented 9 years ago

sigh seems we would also have to check for the availability of Named Blob File.

This is getting pretty ugly, but this would deal with the Named[Blob]File issue:

if HAS_DEXTERITY:
    from plone.namedfile.interfaces import HAVE_BLOBS
    from ftw.builder.dexterity import DexterityBuilder
    if HAVE_BLOBS:
        from plone.namedfile.file import NamedBlobFile as NamedFile
    else:
        from plone.namedfile.file import NamedFile

# ...

    self.attach(NamedFile(data=content, filename=name))
jone commented 9 years ago

@lukasgraf how is HAVE_BLOBS? Looks like it is always True. Which package / build is failing without this change? Can you do the change? I'd like to have this PR merged and released as soon as possible, so that the builds are green again..

jone commented 9 years ago

Let's ship this one; we can fix the namedfile issue separately.

jone commented 9 years ago

I've release 1.5.0.

jone commented 9 years ago

I've fixed the blob issue in #35 as discussed above and released with 1.5.1. (The 1.5.0 release helped me track it down and debug it :wink: )