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

Extract permitted params lists #3336

Merged

Conversation

matiasgarciaisaia
Copy link
Contributor

So they can be easily extended by (...wait for it...) extensions.

@matiasgarciaisaia matiasgarciaisaia force-pushed the feature/extract-permitted-params-list branch 2 times, most recently from 3af8389 to 52292d8 Compare January 16, 2018 19:26
end

def permitted_new_page_part_params
Copy link
Member

@bricesanchez bricesanchez Jan 17, 2018

Choose a reason for hiding this comment

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

This method should be private like all other permitted_* methods in Refinery

matiasgarciaisaia added a commit to manastech/refinerycms that referenced this pull request Jan 17, 2018
@matiasgarciaisaia
Copy link
Contributor Author

👌 Fixed that! I can squash/rebase the commits before merging - made a different commit to help with reviewing.

@bricesanchez
Copy link
Member

@matiasgarciaisaia it will be perfect if you could do this, it's better for tracking changes in the git log! Thank you very much!

So they can be easily extended by (...wait for it...) _extensions_
@matiasgarciaisaia matiasgarciaisaia force-pushed the feature/extract-permitted-params-list branch from 9da0241 to edc4eb2 Compare January 17, 2018 14:22
@matiasgarciaisaia
Copy link
Contributor Author

matiasgarciaisaia commented Jan 17, 2018

There! ☝️

🎉

@bricesanchez bricesanchez self-assigned this Jan 17, 2018
@bricesanchez bricesanchez added this to the 4.0.2 milestone Jan 17, 2018
@bricesanchez bricesanchez merged commit 8baa341 into refinery:master Jan 17, 2018
@bricesanchez
Copy link
Member

Thank you @matiasgarciaisaia !

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