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

refactor filter control panel and tests #2098

Merged
merged 5 commits into from Jul 28, 2017

Conversation

MrTango
Copy link
Contributor

@MrTango MrTango commented Jul 6, 2017

No description provided.

@tisto tisto self-requested a review July 7, 2017 11:21
@@ -37,6 +37,7 @@
'lxml',
'mock',
'plone.app.robotframework>0.9.16',
'robotframework-debuglibrary',
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

@MrTango I don't think this should be declared as a dependency here. I am sure this is already declared somewhere in plone.app.robotframework. cc @datakurre

Copy link
Member

Choose a reason for hiding this comment

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

@tisto It's not a default dependency. "Debug" keyword in p.a.robotframework is a bit fancy. It is dependency with [debug] extras, but that can only be defined in buildout.

  1. open debuglibrary console when that's available
  2. pause with dialog when that dialog is supported by python (it's optional Python build dep)
  3. as last fallback stops with pdb

That said, it's not a perfect dependency. A better option would have been to put it somewhere into buildout.coredev so that's its included in modules of bin/test, but we didn't immediately find the correct testrunner recipe option for that.

@gforcada gforcada force-pushed the midsummersprint_fix_transforms branch from 50c3154 to 2b6515a Compare July 13, 2017 20:59
@gforcada
Copy link
Sponsor Contributor

@MrTango as the branch needed to be rebased, I did it while removing those extra commits of merging with master (pro tip: rebase rather than pull please!).

Before force pushing the branch I double checked that your tree and my rebased tree had exactly the same code bit by bit.

@agitator agitator force-pushed the midsummersprint_fix_transforms branch from 2b6515a to 6dd64f5 Compare July 28, 2017 10:42
@agitator agitator merged commit fdb0ba5 into master Jul 28, 2017
@agitator agitator deleted the midsummersprint_fix_transforms branch July 28, 2017 11:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants