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

fix: correctly merge command class prototypes #765

Merged
merged 2 commits into from
Oct 4, 2024

Conversation

mdonnalley
Copy link
Contributor

Correctly merge static properties up the Command class inheritance chain. Inspired by oclif/core's implementation

Fixes #764
@W-16844507@

@justinwilaby
Copy link

@mdonnalley - Can this be backported down to version 2.x so us dinosaurs can benefit from this fix?

@mdonnalley
Copy link
Contributor Author

@justinwilaby you might be able to ship the heroku cli with plugin-commands v4 without issue. I tested heroku plugins:install @oclif/plugin-commands@^4 and it seems to be working as expected

@justinwilaby
Copy link

justinwilaby commented Sep 26, 2024

@justinwilaby you might be able to ship the heroku cli with plugin-commands v4 without issue. I tested heroku plugins:install @oclif/plugin-commands@^4 and it seems to be working as expected

If so, can this package use peerDependencies to specify a range of compatible @oclif/core versions?

@mdonnalley
Copy link
Contributor Author

If so, can this package use peerDependencies to specify a range of compatible @oclif/core versions?

We can but the acceptable range is all of them - I'm not sure there'd but much value to it. In fact it would probably work even if heroku used the old oclif libraries instead of @oclif/core

@justinwilaby
Copy link

We can but the acceptable range is all of them - I'm not sure there'd but much value to it. In fact it would probably work even if heroku used the old oclif libraries instead of @oclif/core

Great! I suppose the value in the version range would be that implementors would not have yet another version of oclif core installed. The Heroku CLI currently has 3 versions of oclif core in it's dependency tree because of various plugins pinning the version to this one or that one. A version range would prevent this.

@shetzel
Copy link
Contributor

shetzel commented Oct 4, 2024

QA: with your PR branch I now see args such as this for the addons rename command:

"args": {
      "addon_name": {
        "name": "addon_name",
        "required": true
      },
      "new_name": {
        "name": "new_name",
        "required": true
      }
    }

@shetzel shetzel merged commit 9fdb463 into main Oct 4, 2024
11 checks passed
@shetzel shetzel deleted the mdonnalley/merge-prototypes branch October 4, 2024 04:00
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.

args are stripped away when using the global --json flag
3 participants