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

Use namespaced policies #102

Closed
wants to merge 3 commits into from

Conversation

Subtletree
Copy link

@Subtletree Subtletree commented Jul 30, 2018

Proof of concept for namespaced policies #101 #8

If this looks like something that will be merged, I'll finish up with:

  • Finish tests (maybe slim down and just test namespacing rather than duplicating other tests and adding namespace)
  • Add something to the readme.
  • Update method comments

cc @drewnichols

Fix linting errors

fix another linting error
@Subtletree Subtletree force-pushed the namespaced-policies branch from 14e7d75 to e7951b4 Compare July 30, 2018 03:46
@Subtletree
Copy link
Author

Updated minimum pundit version to 2.0 varvet/pundit#529

@jpalumickas
Copy link
Contributor

Is it possible to make this as optional? (Maybe enable/disable in initializers)
For example, we have resources under Api::V1 namespace (Api::V1::UserResource), but our policies are without namespace (UserPolicy), so I think this PR will break, or no?

@valscion
Copy link
Member

valscion commented Aug 3, 2018

@jpalumickas see the discussion in #101 — this PR will definitely not be merged as it currently stands ☺️

@jpalumickas
Copy link
Contributor

@valscion oh! thanks for info 😉 Is it possible easily to upgrade this gem to support Pundit v2? I don't think there was lots of breaking changes there?

@valscion
Copy link
Member

valscion commented Aug 3, 2018

Is it possible easily to upgrade this gem to support Pundit v2? I don't think there was lots of breaking changes there?

Might be! I opened an issue to discuss this further: #103

@valscion valscion added the wip label Aug 7, 2018
@valscion
Copy link
Member

valscion commented Aug 9, 2018

Hi! I'll close this PR for now as this isn't the approach we would like to go with. Feel free to continue the discussion in #103.

Thanks for creating a proof of concept of this approach ❤️

@valscion valscion closed this Aug 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

3 participants