Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Plone 5.2 / also refactor reindex helpers #17

Merged
merged 3 commits into from
Oct 10, 2023

Conversation

reekitconcept
Copy link
Member

@reekitconcept reekitconcept commented Sep 19, 2023

Support Plone 5.2

  • remove Plone dependencies so the package can work with Plone 5.2

  • make constraints work in an alternate way between versions

  • unpin plone.restapi

  • test Plone 5.2 on the CI - currently install only, as test framework
    conflicts with 5.2

Also: Refactor reindex helpers in an importable way

@reekitconcept
Copy link
Member Author

reekitconcept commented Sep 19, 2023

WIP because... I cannot figure out how to run the tests with Plone 5.2 via plone meta.

plone/meta#146 indicates that this cannot currently be done.

I cannot find any way to set this up. There is a way to use an alternate constraints.txt which is what we would need, but I cannot find any way to run two sets of tests, one with the original and one with the 5.2 constrainst. The test job or even the full yaml can be duplicated, but they will still use the same configuration from meta.toml. Am I overlooking something? Any insight is welcome.

EDIT. plone.meta does not support this at the monent, looking for an alternate way.

@reekitconcept reekitconcept force-pushed the ree-support-plone-5.2 branch 9 times, most recently from d1724a1 to b2d4371 Compare September 22, 2023 16:34
@reekitconcept
Copy link
Member Author

reekitconcept commented Sep 25, 2023

Update on Plone 5.2.

The branch works with 5.2, a few caveats:

  • I had to completely drop the dependency of plone.restapi to accomodate both Plone 5.2 and 6, this won't be a problem as soon as the new Plone 6 release requires this version only, until then we may need to add the extra plone.restapi dependency to Plone 6 projects currently use the package.

  • plone.meta does not support testing multiple versions in a matrix, but I added a separate github action

  • the new testing framework seems to have version conflicts with Plone 5.2, I don't think this can be solved - I might be wrong - but in any case we won't add the old tests back, so right now with Plone 5.2 we do test the build but we don't actually run the tests, which is "good enough" at the moment.

- remove Plone dependencies so the package can work with Plone 5.2
- make constraints work in an alternate way between versions
- unpin plone.restapi

- test Plone 5.2 on the CI - currently install only, as test framework
  conflicts with 5.2
@reekitconcept reekitconcept changed the title WIP Support Plone 5.2 Support Plone 5.2 / also refactor reindex helpers Sep 25, 2023
@reekitconcept
Copy link
Member Author

Remark about the reindex helpers: I pushed the update about the reindex helpers here (to make them importable from projects, and use reindex as an upgrade step), so this can go into the next alpha release as well.

.github/workflows/test-plone-5.2.yml Outdated Show resolved Hide resolved
.github/workflows/test-plone-5.2.yml Outdated Show resolved Hide resolved
- name: Install plone
run: make install-plone-5.2

# test
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we have the workflow if we are not running the tests?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At least it runs the install and checks that the dependencies are correct, which is something because that was also breaking earlier.

The main problem is that the test dependencies conflict for me with the Plone 5.2 requirements.

I'd like to run the test but @ericof is it possible to install pytests with Plone 5.2? I'm getting version conflicts. If this is something that is resolvable I'm glad to put more time into it, but it's just a waste of time if the answer is "no".

@reekitconcept
Copy link
Member Author

@ericof I would rely on your help to fix the version conflict for running the tests on Plone 5.2. However I'm also ok to make an alpha release without the tests, I have some upcoming changes anyway so we will definitely have yet another alpha. However we need a release for staging our project that will use this package with Plone 5.2

I leave it up to your consideration, either way (with or without tests) I'll follow up once we have the next alpha.

Copy link
Contributor

@ericof ericof left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merging without 5.2 tests

@ericof ericof merged commit fd5f0b4 into main Oct 10, 2023
6 checks passed
@ericof ericof deleted the ree-support-plone-5.2 branch October 10, 2023 14:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants