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

feat: use hooks for list and display #121

Merged
merged 6 commits into from
Sep 8, 2021
Merged

feat: use hooks for list and display #121

merged 6 commits into from
Sep 8, 2021

Conversation

mdonnalley
Copy link
Contributor

@mdonnalley mdonnalley commented Aug 30, 2021

What does this PR do?

Uses new hook interface from sf-plugins-core for env list and env display

Notable Changes:

  • JSON Schema is now much less specific than it was before. I think this is unavoidable given the federated plugin model
  • Removed --extended flag from env list since we agreed that json output and table output will always have the same information

What issues does this PR fix or reference?

@W-9811480@
@W-9769891@

@mdonnalley mdonnalley self-assigned this Aug 30, 2021
@mdonnalley mdonnalley force-pushed the mdonnalley/use-hooks branch from 7339d47 to d5dd6e4 Compare August 30, 2021 20:19
peternhale
peternhale previously approved these changes Aug 31, 2021
Copy link
Contributor

@peternhale peternhale left a comment

Choose a reason for hiding this comment

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

@mdonnalley just one suggestion to replace if/else with lambda

Comment on lines 63 to 59
for (const org of orgs) {
if (org.isScratchOrg) {
grouped.scratchOrgs = grouped.scratchOrgs.concat(org);
} else {
grouped.nonScratchOrgs = grouped.nonScratchOrgs.concat(org);
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
for (const org of orgs) {
if (org.isScratchOrg) {
grouped.scratchOrgs = grouped.scratchOrgs.concat(org);
} else {
grouped.nonScratchOrgs = grouped.nonScratchOrgs.concat(org);
}
}
[scratchOrgs, nonScratchOrgs] = orgs.reduce(([s,n], o) => o.isScratchOrg() ? [[...s, o], n] : [s, [...n, o]], [[],[]]
const grouped = {
nonScratchOrgs: nonScratchOrgs as OrgAuthorization[],
scratchOrgs: scratchOrgs as OrgAuthorization[],
};

peternhale
peternhale previously approved these changes Sep 1, 2021
src/hooks/envList.ts Outdated Show resolved Hide resolved
RodEsp
RodEsp previously approved these changes Sep 2, 2021
@RodEsp
Copy link
Contributor

RodEsp commented Sep 2, 2021

Heya @mdonnalley I'm QA'ing this now and noticed sf env list hangs at the end. I assume this is to do with the hooks, is there anything that could be done about that?

sfEnvListHookHanging

@mdonnalley mdonnalley dismissed stale reviews from RodEsp and peternhale via a19aa7e September 3, 2021 16:35
peternhale
peternhale previously approved these changes Sep 7, 2021
@mdonnalley
Copy link
Contributor Author

Heya @mdonnalley I'm QA'ing this now and noticed sf env list hangs at the end. I assume this is to do with the hooks, is there anything that could be done about that?

sfEnvListHookHanging

This will be resolved by oclif/core#243

@RodEsp
Copy link
Contributor

RodEsp commented Sep 8, 2021

Heya @mdonnalley I'm QA'ing this now and noticed sf env list hangs at the end. I assume this is to do with the hooks, is there anything that could be done about that?
...

This will be resolved by oclif/core#243

I'm still seeing the same delay after pulling and building the latest changes and running sf env list (even without my test plugin, which was updated too).

@RodEsp
Copy link
Contributor

RodEsp commented Sep 8, 2021

The delay at the end was only happening when using node v16.8.0. Switching to v16.9.0 got rid of the issue so going to go ahead and merge this.

@RodEsp RodEsp merged commit 5b56b8a into main Sep 8, 2021
@RodEsp RodEsp deleted the mdonnalley/use-hooks branch September 8, 2021 14:36
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