Refactor pundit policy and policy_scope

varvet / pundit

Minimal authorization through OO design and pure Ruby classes

MIT License

8.21k stars 624 forks source link

Refactor pundit policy and policy_scope #787

Closed bhtabor closed 8 months ago

bhtabor commented 8 months ago

Refactor pundit policy and policy_scope

Burgestrand commented 8 months ago

Hi! Thank you for the PR.

For me, refactoring needs to have a goal. Either by making something easier to implement, or something easier to understand.

This refactor:

Doesn't implement anything, so the refactor is not in order to "make the change easy".
It adds indirection, which makes code harder to understand.

It does indeed reduce duplication which could be a good thing, but in this PR it's not apparent what that might be. That's why I'm closing this without merge.

bhtabor commented 8 months ago

For me, refactoring needs to have a goal. Either by making something easier to implement, or something easier to understand.

Hi @Burgestrand, thank you for reviewing. The reason I chose to send a refactor PR was because the duplicate error handling in the bang methods made it seem like error handling was different for the bang methods.