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

Optional registry #486

Merged
merged 16 commits into from
Nov 6, 2020
Merged

Optional registry #486

merged 16 commits into from
Nov 6, 2020

Conversation

tdamsma
Copy link

@tdamsma tdamsma commented Oct 25, 2020

Implements changes suggested in #485.

I already put it up here for review, but did not properly document the changes yet.

@stevepiercy
Copy link
Member

This is looking good! Just a couple more lines to cover. Lint and docs don't like the docstrings, so I'll suggest improvements to fix those jobs.

Copy link
Member

@stevepiercy stevepiercy left a comment

Choose a reason for hiding this comment

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

These changes should get lint and docs to pass.

deform/widget.py Outdated Show resolved Hide resolved
deform/widget.py Outdated Show resolved Hide resolved
@stevepiercy
Copy link
Member

@tdamsma the functional tests started failing after the merge commit 51b5418. Can you take a look to see what needs fixing?

@tdamsma
Copy link
Author

tdamsma commented Nov 3, 2020

yes, I'll have a look. I switched computer three times in the last week, so need to take a moment to get a proper environment up and running. I can't see why/how 51b5418 could have changed anything for the functional tests though

@tdamsma tdamsma requested a review from stevepiercy November 3, 2020 15:18
@stevepiercy
Copy link
Member

@tdamsma I approve your work. Would you please review my docstring improvements and documentation additions in the last commit? I think I got it right, but it helps to have a second pair of eyes. Thank you!

docs/widget.rst Outdated Show resolved Hide resolved
docs/widget.rst Outdated Show resolved Hide resolved
deform/widget.py Outdated Show resolved Hide resolved
Copy link
Author

@tdamsma tdamsma left a comment

Choose a reason for hiding this comment

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

Nice improvements, please go ahead and finish it. I put some minor comments, feel free to ignore them

docs/widget.rst Outdated Show resolved Hide resolved
deform/widget.py Show resolved Hide resolved
deform/widget.py Outdated Show resolved Hide resolved
@stevepiercy
Copy link
Member

I think we're done. w00t! Thank you @tdamsma!

@stevepiercy stevepiercy merged commit 08d642c into Pylons:2.0-branch Nov 6, 2020
@tdamsma tdamsma deleted the optional-registry branch November 6, 2020 11:20
stevepiercy added a commit to stevepiercy/deform that referenced this pull request Nov 6, 2020
stevepiercy added a commit that referenced this pull request Nov 6, 2020
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.

2 participants