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 authconfig support #387

Merged
merged 1 commit into from Jul 13, 2021
Merged

remove authconfig support #387

merged 1 commit into from Jul 13, 2021

Conversation

pbrezina
Copy link
Contributor

@pbrezina pbrezina commented Jul 9, 2021

Authconfig compatibility tool (from authselect-compat) will be removed from Fedora 35:
https://fedoraproject.org/wiki/Changes/RemoveAuthselectCompatPackage

@bcl
Copy link
Collaborator

bcl commented Jul 9, 2021

This isn't the correct way to remove a command. We have a class for that :) See commit ab95215 for an example.

@pbrezina
Copy link
Contributor Author

Thanks for the hint. See new patch.

pykickstart/commands/authconfig.py Outdated Show resolved Hide resolved
Authconfig compatibility tool (from authselect-compat) will be removed from Fedora 35:
https://fedoraproject.org/wiki/Changes/RemoveAuthselectCompatPackage
Copy link
Collaborator

@VladimirSlavik VladimirSlavik left a comment

Choose a reason for hiding this comment

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

Looks good to me, thank you!

Copy link
Collaborator

@bcl bcl left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@bcl bcl merged commit e65e786 into pykickstart:master Jul 13, 2021
@VladimirSlavik
Copy link
Collaborator

@pbrezina, I'm curious, why did you make the initial patch just a plain removal of stuff from the handler?

The reason to ask is, I re-checked the docs for removing a command and noticed that's mentioned there as the first thing. The "correct new way" is mentioned as an alternative. So if that text was the reason for how this looked, then amending the doc would be a good idea.

@pbrezina
Copy link
Contributor Author

Honestly, I was just too lazy and did not check any docs.

@VladimirSlavik
Copy link
Collaborator

Fair enough!

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.

None yet

3 participants