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

Move usergroups controlpanel (now using z3cform) from p.a.controlpanel to Products.CMFPlone #262

Merged
merged 10 commits into from
Dec 22, 2014

Conversation

vanclevstik
Copy link
Contributor

  • Moved usergroups controlpanel
  • Transformed doctests into unit tests and added some more unit tests for different edge cases
  • Added robot test for each part of usergroups controlpanel

Refs #221

@jensens
Copy link
Member

jensens commented Sep 2, 2014

Looks overall good as far as I can say. Can you please add a line or two in order to log the changes in CHANGES.rst?

@jensens
Copy link
Member

jensens commented Sep 9, 2014

This is ready for merge.

But in https://github.com/plone/plone.app.controlpanel/tree/master/plone/app/controlpanel we have still the usergroups code in. Whats the plan here?

@tisto
Copy link
Member

tisto commented Sep 9, 2014

We need two pull requests. One for CMFPlone with the new control panel and one for p.a.controlpanel removing the control panel. Someone needs to run "bin/alltests --all" with those two pull requests checked out locally to make sure we do not break Jenkins.

@jensens Github says this pull request is not ready for merge. I'd be happy to review if there are two pull request and "bin/alltests --all" passes.

@vanclevstik
Copy link
Contributor Author

Sorry I was a bit busy these last days and didn't have time to fix/change this.
I will rebase the PR, make a PR in p.a.controlpanel and check if tests are still passing!

@tisto
Copy link
Member

tisto commented Sep 9, 2014

@ferewuz Awesome! Thanks! Let me know if you need any help.

@jensens
Copy link
Member

jensens commented Sep 9, 2014

oh, sorry, did the rebase already. great if a PR in p.a.controlpanel comes too!

@vanclevstik
Copy link
Contributor Author

Unfortunately some tests seem to be failing right now, I'll check it some more to find out what the problems are.

@tisto
Copy link
Member

tisto commented Nov 23, 2014

@ferewuz What's the current status of this pull request? Would you mind doing a rebase? Which tests are failing? Anything I can do to help?

@vanclevstik
Copy link
Contributor Author

Sorry for a bit late response for this one. I will find some time today in the evening, do a rebase and check the tests again.

@vanclevstik vanclevstik force-pushed the usergroups_controlpanel branch from 28791dc to 97b2d07 Compare November 27, 2014 22:23
@vanclevstik
Copy link
Contributor Author

@tisto Rebased now and tests for Products.CMFPlone are ok, but i get some errors in other packages, can you take a quick look when you have some time?

@tisto
Copy link
Member

tisto commented Dec 1, 2014

@ferewuz Thx. Will have a look.

@tisto
Copy link
Member

tisto commented Dec 22, 2014

New pull request for plone.app.controlpanel: plone/plone.app.controlpanel#37

tisto added a commit that referenced this pull request Dec 22, 2014
Move usergroups controlpanel (now using z3cform) from p.a.controlpanel to Products.CMFPlone
@tisto tisto merged commit 1c59266 into master Dec 22, 2014
@tisto tisto deleted the usergroups_controlpanel branch December 22, 2014 10:26
jensens pushed a commit that referenced this pull request Nov 12, 2021
Remove cyclic dependency with Products.CMFPlone.
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