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.

Sign up for GitHub

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

Jump to bottom

Print total results banner at the end of execution #160

Merged
merged 3 commits into from
Oct 1, 2019
Merged

Print total results banner at the end of execution #160

merged 3 commits into from
Oct 1, 2019

Conversation

budevg
Copy link
Contributor

@budevg budevg commented Sep 28, 2019

Description

Simple results banner which is printed at the end

Fixes/Implements: # (issue)

This commit resolves #155

New dependencies introduced?

no

Checklist:

  • [ x ] I have added tests that prove my fix is effective or that my feature works
  • [ x ] I have update README (If needed)

@welcome
Copy link

welcome bot commented Sep 28, 2019

🙏 Thanks for opening this pull request! Please check out our contributing guidelines.

@auto-comment
Copy link

auto-comment bot commented Sep 28, 2019

🙏 Thank your for raising your pull request.
Please make sure you have followed our contributing guidelines. We will review it as soon as possible

@gopinath-langote
Copy link
Owner

@budevg Please make sure your fork is up-to-date with the master.

@gopinath-langote gopinath-langote added new_feature New features In Progress Someone working on the issue Hacktoberfest Support to https://hacktoberfest.digitalocean.com/ labels Sep 30, 2019
@gopinath-langote gopinath-langote added this to the v1.4.0 milestone Sep 30, 2019
@budevg
Copy link
Contributor Author

budevg commented Sep 30, 2019

ok I rebased the fork and force pushed the pr

@gopinath-langote
Copy link
Owner

Your current implementation prints following

------------------------------------------------------------------------
SUCCESS - Total Time: 6m28.367694s
------------------------------------------------------------------------

and

------------------------------------------------------------------------
SUCCESS - Total Time: 11.232510756s
------------------------------------------------------------------------

Let follow the guideline specified in issue

Format as XXm YYs – If XX = 00 only print YYs
ex,

  1. 01m 45s
  2. 04s
  3. 21s
  • Also, round the seconds ex

11.232510756s to 11s
9.562334 to 10s

Round down for - <0.5 & round uo for >0.5

gopinath-langote
gopinath-langote requested changes Oct 1, 2019
View reviewed changes
cmd/exec/exec.go
}

func executeAndStopIfFailed(command *models.CommandContext) {
func executeAndStopIfFailed(command *models.CommandContext, executeStart time.Time) {
command.PrintBanner()
err := command.CommandSession.Run()
if err != nil {
exitCode := (err.Error())[12:]
Copy link
Owner

Choose a reason for hiding this comment

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

The only thing missing is removing the dashed banner from the failure message.
Screenshot 2019-10-01 at 17 43 13

Copy link
Owner

Choose a reason for hiding this comment

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

Check issue description #155 for more details.

Copy link
Owner

@gopinath-langote gopinath-langote Oct 1, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

With message Execution failed: phase 'a' - exit code: 127

phase and exit code could be anything

@gopinath-langote
Copy link
Owner

@budevg Thanks and congratulations on the first PR for #Hacktoberfest

@gopinath-langote gopinath-langote merged commit e96cc7b into gopinath-langote:master Oct 1, 2019
@welcome
Copy link

welcome bot commented Oct 1, 2019

Congrats on merging your first pull request! We here at 1build are proud of you! 🙏

themayurkumbhar pushed a commit to themayurkumbhar/1build that referenced this pull request Oct 8, 2019
@budevg @themayurkumbhar
Print total results banner at the end of execution (gopinath-langote#160
5dd2e3f 
)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gopinath-langote gopinath-langote gopinath-langote approved these changes

Assignees

@budevg budevg

Labels
Hacktoberfest Support to https://hacktoberfest.digitalocean.com/ In Progress Someone working on the issue new_feature New features
Projects
None yet
Milestone
v1.4.0
Development

Successfully merging this pull request may close these issues.

Print total results banner at the end of execution
2 participants
@budevg @gopinath-langote