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

Keyword arguments in path handlers #2585

Merged
merged 2 commits into from Dec 7, 2016
Merged

Keyword arguments in path handlers #2585

merged 2 commits into from Dec 7, 2016

Conversation

@Kwpolska
Copy link
Member

Kwpolska commented Dec 6, 2016

This implements the generic functionality needed for #2580. I’ll try to do it for the Taxonomy stuff in a future PR (first, I need to understand the thing)

cc @felixfontein

Kwpolska added 2 commits Dec 6, 2016
Signed-off-by: Chris Warrick <kwpolska@gmail.com>
Signed-off-by: Chris Warrick <kwpolska@gmail.com>
Copy link
Contributor

felixfontein left a comment

LGTM

@felixfontein
Copy link
Contributor

felixfontein commented Dec 6, 2016

The taxonomy plugin's path handler for indexes can already handle page numbers when they are returned from a taxonomy's get_path. So changing TaxonomiesClassifier._taxonomy_path to handle an optional page argument should be not too hard.

@Kwpolska Kwpolska merged commit d743c67 into master Dec 7, 2016
5 checks passed
5 checks passed
codacy/pr Good work! A positive pull request.
Details
continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@Kwpolska Kwpolska deleted the kwargs-in-path-handlers branch Dec 7, 2016
@felixfontein
Copy link
Contributor

felixfontein commented Dec 8, 2016

Do you still want to do the future PR, or should I throw something together?

@Kwpolska
Copy link
Member Author

Kwpolska commented Dec 8, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.