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 unused Knob function #235

Closed
wants to merge 1 commit into from
Closed

Remove unused Knob function #235

wants to merge 1 commit into from

Conversation

MartinBasti
Copy link
Contributor

@MartinBasti MartinBasti commented Nov 11, 2016

Knob function is outdated and was replaced by knob. Make explicit
note in code about this.

https://fedorahosted.org/freeipa/ticket/6392

@stlaz
Copy link
Contributor

stlaz commented Nov 11, 2016

ACK, there should be note about this deprecation somewhere. Deleting Knob might be worth a ticket as well.

@stlaz stlaz added the ack Pull Request approved, can be merged label Nov 11, 2016
@MartinBasti
Copy link
Contributor Author

I would wait with ACK, I realized that Knobs with capital K are not used anymore, so we can remove it instead of deprecating

@MartinBasti MartinBasti removed the ack Pull Request approved, can be merged label Nov 11, 2016
`Knob` function is an old implementation which was replcaed by `knob`
function and currently is unused, so it can be removed

https://fedorahosted.org/freeipa/ticket/6392
@MartinBasti MartinBasti changed the title Make Knob function deprecated Remove unused Knob function Nov 11, 2016
@stlaz
Copy link
Contributor

stlaz commented Nov 13, 2016

From our offline discussion I got the impression the Knob function was still used somewhere, therefore the ACK. I'm not sure what was the reason of keeping Knob there even if unused, you may need checking with @jcholast.

@MartinBasti
Copy link
Contributor Author

@jcholast ping

@HonzaCholasta
Copy link
Contributor

There was no reason, I just forgot, so go ahead.

@MartinBasti MartinBasti added ack Pull Request approved, can be merged pushed Pull Request has already been pushed labels Nov 22, 2016
@MartinBasti
Copy link
Contributor Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ack Pull Request approved, can be merged pushed Pull Request has already been pushed
Projects
None yet
3 participants