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: improve handling of pvc path flag #129

Merged
merged 12 commits into from
Oct 24, 2022
Merged

fix: improve handling of pvc path flag #129

merged 12 commits into from
Oct 24, 2022

Conversation

peternhale
Copy link
Contributor

What does this PR do?

Improves handling of the path flag for package version create

What issues does this PR fix or reference?

@W-11903179@ GH issue

@peternhale peternhale requested a review from shetzel October 17, 2022 12:08
@peternhale peternhale requested a review from a team as a code owner October 17, 2022 12:08
Copy link
Contributor

@shetzel shetzel left a comment

Choose a reason for hiding this comment

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

Looks to me like we can remove a bunch of code in package beta version create command and rely on the library.

package.json Outdated Show resolved Hide resolved
@@ -221,4 +209,24 @@ export class PackageVersionCreateCommand extends SfdxCommand {
}
return result;
}

private resolvePackageIdFromFlags(): string {
Copy link
Contributor

Choose a reason for hiding this comment

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

PackageVersion.create() handles all of this already right? That's what I see when looking at library code for packageVersionCreate.packageVersionCreate().

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It should (there is a bug in that code), but I really do not like passing a path as an id or alias. To me it is just making the determination of which package much more difficult.

@peternhale peternhale merged commit e52f9ec into main Oct 24, 2022
@peternhale peternhale deleted the phale/W-11903179 branch October 24, 2022 12:06
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.

3 participants