-
Notifications
You must be signed in to change notification settings - Fork 151
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
Fix orgy #113
Fix orgy #113
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It sure is awesome that we have no CI set up for this repo 😬
@dpogue Oh, nevermind the CI. There are no tests anyway 😋 Okay, I'm being unfair here. There is one test case. And it tested the parsing of options that should not even exist before I fixed it here 😁 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Instead of --plugin_{id,version} we had --platform_{id,version} As unknown options are always interpreted as Boolean, before this fix the created plugin.xml always contains <plugin id="true" version="true"> See also apache#110 (comment)
Motivation and Context
Plugman is horribly broken in so many aspects. This PR fixes some of the bugs. All changes are only patch level IMO.
Fixes #111
Description
See commits for details.
Testing
I did manual testing of all but the
install
anduninstall
commands. The one existing test case passes.