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

Deep tree handler customization support #265

Merged
merged 2 commits into from
May 7, 2019
Merged

Conversation

idlesign
Copy link
Owner

@idlesign idlesign commented May 1, 2019

No description provided.

@coveralls
Copy link

coveralls commented May 1, 2019

Coverage Status

Coverage increased (+0.02%) to 94.925% when pulling a533695 on feat/deep_customization into 61de460 on master.

Copy link

@go-run-jump go-run-jump left a comment

Choose a reason for hiding this comment

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

Nice work! That makes it easy to do the kind of stuff I want to do. I could get the demo running with django-guardian.

with the help of `SITETREE_CLS` setting.

1. Subclass `sitetreeapp.SiteTree` and place that class into a separate module for convenience.
2. Override methods you need for customization (usually `.apply_hook()`).

Choose a reason for hiding this comment

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

From that description I don't really understand how .apply_hook() comes into play.

Copy link
Owner Author

@idlesign idlesign May 7, 2019

Choose a reason for hiding this comment

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

.appy_hook() is most generic — this description doesn't necessarily cover your very case from #263
Rather you override .get_permissions().

Choose a reason for hiding this comment

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

Yeah, thats right. I just wanted to point out, that somebody who reads this sentence might not understand if he has to overwrite .apply_hook() or if this is just one option to deal with a usecase.

But I guess if people are doing such things they must have a look at the code and then everything is pretty self-explanatory anyways.

Copy link
Owner Author

Choose a reason for hiding this comment

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

I see. If you can propose a better wording, I'd be glad.

Choose a reason for hiding this comment

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

I'd be a little bit more explicit, like this:

You can now overwrite .apply_hook() to manipulate values at pre-defined hooks. You can also overwrite any other methods to customize to your exact needs.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Done. Thank you.

@idlesign idlesign merged commit fad5423 into master May 7, 2019
@idlesign idlesign deleted the feat/deep_customization branch May 7, 2019 12:58
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

3 participants