Skip to content
This repository has been archived by the owner on Jul 12, 2022. It is now read-only.

Set tutorial output more informative #583

Merged
merged 2 commits into from
Oct 5, 2020

Conversation

Harirai
Copy link
Contributor

@Harirai Harirai commented Oct 3, 2020

Signed-off-by: Harirai harim1709@gmail.com
Fixes #525

- What I did
Added user response to rit tutorial for more clarity, previously output was "Set tutorial successful" irrespective of user response.
Now it shows output corresponding to the input.
Like: "Set tutorial enabled successful".
- How to verify it
By setting tutorial.
- Description for the changelog
Set tutorial output more informative.

Signed-off-by: Harirai <harim1709@gmail.com>
@codecov-commenter
Copy link

codecov-commenter commented Oct 3, 2020

Codecov Report

Merging #583 into master will decrease coverage by 0.34%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #583      +/-   ##
==========================================
- Coverage   79.89%   79.55%   -0.35%     
==========================================
  Files          98       98              
  Lines        3253     3218      -35     
==========================================
- Hits         2599     2560      -39     
- Misses        466      469       +3     
- Partials      188      189       +1     
Impacted Files Coverage Δ
pkg/cmd/tutorial.go 100.00% <100.00%> (ø)
pkg/formula/tree/default_tree.go 81.81% <0.00%> (-11.52%) ⬇️

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 f1b539e...092f218. Read the comment docs.

@@ -68,7 +68,7 @@ func (o tutorialCmd) runStdin() CommandRunnerFunc {
return err
}

prompt.Success("Set tutorial successful!")
prompt.Success("Set tutorial " + obj.Tutorial + " successful!")
Copy link
Contributor

Choose a reason for hiding this comment

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

what you think about

prompt.Success("Tutorial " + response + "d")

for me it's a clearer message

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah sure!
I assume you want message to be like "Tutorial enabled" , response already contains "d".
So I think it should be prompt.Success("Tutorial " + response) .
Tell me if I understood it wrong otherwise I'll make the above changes and fix the test soon. 😃

Copy link
Contributor

Choose a reason for hiding this comment

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

that's it @Harirai
keep going

@victor-schumacher
Copy link
Contributor

i see there are functional tests failing
to fix this update the file bellow with the new tutorial text:

"result": "Set tutorial successful!"

@lucasdittrichzup lucasdittrichzup added 🔨 improvement Improvement in features ✔️ ready-for-review ready for review Hacktoberfest https://hacktoberfest.digitalocean.com/ labels Oct 5, 2020
Signed-off-by: Harirai <harim1709@gmail.com>
@henriquemoraeszup henriquemoraeszup merged commit 5855b88 into ZupIT:master Oct 5, 2020
@Harirai Harirai deleted the fix-tutoral-msg branch October 8, 2020 13:47
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Hacktoberfest https://hacktoberfest.digitalocean.com/ 🔨 improvement Improvement in features ✔️ ready-for-review ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

'rit tutorial' command shows the same output for enabled or disabled
5 participants