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

Adding timestamps to command executions in output… #780

Merged
merged 1 commit into from
Dec 7, 2018

Conversation

lcampos
Copy link
Contributor

@lcampos lcampos commented Nov 29, 2018

… view

What does this PR do?

Adds timestamps to every sfdx command execution that's being printed in the Output panel.

As the title suggests, this is a work in progress cause I want to run the automation and see if I missed any test failures and get feedback.

What issues does this PR fix or reference?

#759

timestamp_preview_highlight

@codecov
Copy link

codecov bot commented Nov 29, 2018

Codecov Report

Merging #780 into develop will increase coverage by 0.02%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #780      +/-   ##
===========================================
+ Coverage    72.71%   72.73%   +0.02%     
===========================================
  Files          167      167              
  Lines         6772     6779       +7     
  Branches      1064     1065       +1     
===========================================
+ Hits          4924     4931       +7     
  Misses        1582     1582              
  Partials       266      266
Impacted Files Coverage Δ
...forcedx-vscode-core/src/channels/channelService.ts 85.71% <100%> (+2.85%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ba324ca...b83e73b. Read the comment docs.

@@ -79,6 +85,19 @@ export class ChannelService {
});
}

private getExecutionTime() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using the same time format as we're using in apex lsp and debug logs.


execution.processExitSubject.subscribe(data => {
this.channel.append(execution.command.toCommand());
this.channel.append(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I looked at having this as part of the classes in commandExecutor.ts, having an internal variable (per class) to track start and end of execution and then leverage them whenever we wanted to print them. It turned out to be more complex and un-intuitive in case we wanted to use those timestamps somewhere else other than for the output view.
I then turned to the simplest approach which is adding it here on the right execution methods. Let me know if I should look at moving this somewhere else where it makes more sense.

cc @vazexqi

@lcampos lcampos force-pushed the lcampos/timestamps-on-output-lines branch from da0db58 to 11cec0c Compare November 30, 2018 22:33
@lcampos lcampos changed the title [WIP] Initial approach to adding timestamps to command executions in output… Adding timestamps to command executions in output… Dec 3, 2018
@lcampos lcampos force-pushed the lcampos/timestamps-on-output-lines branch from 11cec0c to 0676a73 Compare December 5, 2018 19: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.

3 participants