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

Remove duplicated method #365

Merged
merged 1 commit into from
Dec 31, 2017
Merged

Conversation

printercu
Copy link
Contributor

It's redefined on L141.

Copy link
Collaborator

@tbrisker tbrisker left a comment

Choose a reason for hiding this comment

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

I'd suggest removing the protected version instead, in case some consumer uses this method.

@printercu
Copy link
Contributor Author

I can do this. But this one is protected now, so one can use it only with send.

Copy link
Collaborator

@tbrisker tbrisker left a comment

Choose a reason for hiding this comment

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

Right, now if some consumer of the gem expects the non_audited_columns method to be public this change will break their code, hence it would be better to remove the protected once and not the public one.

Copy link
Collaborator

@tbrisker tbrisker left a comment

Choose a reason for hiding this comment

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

Oh, now I see what you mean, it is already protected because of the duplicate definition. In that case makes sense to remove the public version.

@tbrisker tbrisker merged commit e1dd759 into collectiveidea:master Dec 31, 2017
@tbrisker
Copy link
Collaborator

Thanks @printercu !

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