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: add progress bar to deploy #65

Merged
merged 1 commit into from
Apr 9, 2021
Merged

fix: add progress bar to deploy #65

merged 1 commit into from
Apr 9, 2021

Conversation

WillieRuemmele
Copy link
Contributor

What does this PR do?

Adds a progress bar for deploy will count the number of components deployed, and tests ran

should behave identically to toolbelt

What issues does this PR fix or reference?

@W-9014708@

@WillieRuemmele WillieRuemmele requested a review from a team April 8, 2021 20:07
});

// if SFDX_USE_PROGRESS_BAR is true and no --json flag use progress bar, if not, skip
if (env.getBoolean('SFDX_USE_PROGRESS_BAR', true) && !this.flags.json) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I thought the default was to use the progress bar?
I've never set SFDX_USE_PROGRESS_BAR on my local but the progress bar does show up.

Copy link
Contributor

Choose a reason for hiding this comment

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

@RodEsp that's what this is. If the env var is unset default to true.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, got'cha. I didn't realize that's what getBoolean did, thanks!
But in that case can we change the comment please?

// if SFDX_USE_PROGRESS_BAR is true or not set and no --json flag use progress bar, otherwise skip

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@RodEsp we're going to merge this as is, but we need to move the progress bar logic and I'll update this comment when I do that

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.

None of my comments are blocking. LGTM!

import { Duration } from '@salesforce/kit';
import { asString, asArray, getBoolean, JsonCollection } from '@salesforce/ts-types';
import * as chalk from 'chalk';
import cli from 'cli-ux';
Copy link
Contributor

Choose a reason for hiding this comment

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

This is supposed to be available on this when you extend SfdxCommand but it's a different library version. I wonder if we have a story to bump the version in @salesforce/command so we don't have to import the version we need for the progress bar.

import { SourceCommand } from '../../../sourceCommand';

Messages.importMessagesDirectory(__dirname);
const messages = Messages.loadMessages('@salesforce/plugin-source', 'deploy');

type TestLevel = 'NoTestRun' | 'RunSpecifiedTests' | 'RunLocalTests' | 'RunAllTestsInOrg';
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a type in SDR. We should ask them to export it so we don't have to redefine it.

});

// if SFDX_USE_PROGRESS_BAR is true and no --json flag use progress bar, if not, skip
if (env.getBoolean('SFDX_USE_PROGRESS_BAR', true) && !this.flags.json) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@RodEsp that's what this is. If the env var is unset default to true.


it('should NOT call progress bar because of environment variable', async () => {
try {
process.env.SFDX_USE_PROGRESS_BAR = 'false';
Copy link
Contributor

Choose a reason for hiding this comment

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

Nothing wrong with this but jsyk you can stub env.getBoolean instead of wrapping in try/finally

});

// if SFDX_USE_PROGRESS_BAR is true and no --json flag use progress bar, if not, skip
if (env.getBoolean('SFDX_USE_PROGRESS_BAR', true) && !this.flags.json) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, got'cha. I didn't realize that's what getBoolean did, thanks!
But in that case can we change the comment please?

// if SFDX_USE_PROGRESS_BAR is true or not set and no --json flag use progress bar, otherwise skip

@shetzel shetzel merged commit df2f5e0 into main Apr 9, 2021
@shetzel shetzel deleted the wr/progressbar branch April 9, 2021 17:18
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.

4 participants