Skip to content

W-22164863: Address review comments and update backend request param#11

Merged
ukanoja-sf merged 3 commits into
mainfrom
t/tm-connect/update-contract-and-fit-n-finish
Apr 29, 2026
Merged

W-22164863: Address review comments and update backend request param#11
ukanoja-sf merged 3 commits into
mainfrom
t/tm-connect/update-contract-and-fit-n-finish

Conversation

@ukanoja-sf
Copy link
Copy Markdown
Collaborator

What does this PR do?

  1. Address review comments from here: a3b7112
  2. Update backend request param from namespacePrefix to namespace

What issues does this PR fix or reference?

GUS: @W-22164863@

Local testing

image image image image image

Copy link
Copy Markdown
Collaborator

@acardel acardel 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, I only left one comment.

Comment thread README.md
Replace `<package-version-id>` with the 04t ID of the package version you want to test.

```bash
sf package install --package <package-version-id> --target-org <scratch-org-alias>
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@ukanoja-sf ,
I see that in cli we only mention the flow of installing a package.
The other valid flow is to do sf project deploy start to deploy the project, devs don't have to release a new package when developing.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Updated the steps

Comment thread README.md
sf package install --package <package-version-id> --target-org <scratch-org-alias>
```

**Push source directly**
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I would say "OR Push source directly"

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I think we don't need to mention OR as text above mentions it by stating Make the package available in the scratch org. Some ways to do this include:

char: 'l',
summary: messages.getMessage('flags.license.summary'),
exclusive: ['definition-file'],
relationships: [{ type: 'all', flags: ['namespace', 'quantity'] }],
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Eric mentioned this in his review....
Can we either:

  1. Remove the dependsOn and just use the relationships: [{ type: 'all', flags: ..... property for the namespace, license and quantity flags
    OR
  2. Remove the relationships property for license and use the dependsOn property for the namespace, license and quantity flags

Doesn't matter to me which one, we should just be consistent.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

done

Comment thread messages/license.provision.md Outdated
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

done

@ukanoja-sf ukanoja-sf merged commit 14b5c5b into main Apr 29, 2026
14 checks passed
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