search

plone / plone.i18n

Text normalization logic and language, country, cctld data.
8 stars 11 forks source link

Break circular reference by moving interface to this package. #30

Closed sallner closed 5 years ago

sallner commented 5 years ago

Fixes #29 .

Explicitly declare dependencies which have been there implicitly by depending on Products.CMFPlone.

The interface should be imported in Products.CMFPlone.interfaces after the merge.

mister-roboto commented 5 years ago

@sallner thanks for creating this Pull Request and help improve Plone!

To ensure that these changes do not break other parts of Plone, the Plone test suite matrix needs to pass.

Whenever you feel that the pull request is ready to be tested, either start all jenkins jobs pull requests by yourself, or simply add a comment in this pull request stating:

@jenkins-plone-org please run jobs

With this simple comment all the jobs will be started automatically.

Happy hacking!

sallner commented 5 years ago

@jenkins-plone-org please run jobs

jensens commented 5 years ago

Is there a PR for Products.CMFPlone already?

sallner commented 5 years ago

Is there a PR for Products.CMFPlone already?

@jensens No, I actually do not know the procedure to do this, as I would need both branches, here and in plone/Products.CMFPlone So where would be the correct place to do this? My plone contributor skills are not fully developed yet.

Should I leave this PR open till the one on CMFPlone was successful?

jensens commented 5 years ago

Open two PR's and then log in to Jenkins and fill in both PRs into the fields at https://jenkins.plone.org/job/pull-request-5.2/build?delay=0sec (and for 5.2-3.6 and 5.2-3.7 jobs as above linked in the checks)

gforcada commented 5 years ago

Actually, as this pull request alone can be merged, tests do not break seems, we can merge this one and shortly after create the other in CMFPlone :+1:

Or better, prepare the other pull request, and once it has been reviewed (ignore the test failures), let's merge this one and then test the other.