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: Show a completion message when watching #253

Merged
merged 1 commit into from
Dec 28, 2019
Merged

feat: Show a completion message when watching #253

merged 1 commit into from
Dec 28, 2019

Conversation

codehearts
Copy link
Contributor

The completion message is shown only once all exercises succeed and are
not annotated with "I AM NOT DONE." The watch command will also exit

closes #251

Let me know if there are any tests I could add or if the completion message should be tweaked!

@AbdouSeck
Copy link
Contributor

AbdouSeck commented Dec 23, 2019

Hi @codehearts,

Thanks for the PR. I noticed that the src/verifiy.rs module was changed to add println! lines in the verify function, and also std::process::exit(0) is used inside the watch function in main.rs. While that's great, both the printing and process exiting could have been done inside the main function. Since both the verify and watch functions have clear return types and values, I thought the following would be easier:

  • Have no printing in the module verify since it can be used for something that may not require any printing
  • Have watch return Ok(()) whenever verify_result.is_ok() instead of std::process::exit(0) (since the function can be used for something else that may not require process exiting)

Doing the above makes it so that you can do the following inside the main function:

if matches.subcommand_matches("watch").is_some() {
        if let Ok(()) = watch(&exercises) {
            println!("Done with all exercises.");
            // More printing
            std::process::exit(0);
        }
    }

Please keep in mind that these are just suggestions. You're welcome to keep the PR intact. I just thought it might help to move process exiting steps into the main function, so that it is clear that main is the driver of the binary.

Thank you,

Abdou

@codehearts
Copy link
Contributor Author

I agree with your suggestions, I’ll rebase with that implementation as soon as I can

The completion message is shown only once all exercises succeed and are
not annotated with "I AM NOT DONE." The watch command will also exit

closes #251
@codehearts
Copy link
Contributor Author

I've rebased your suggestions into the original commit, since I think your approach was better. Let me know if there's anything else you see that might need some tweaking!

@AbdouSeck
Copy link
Contributor

AbdouSeck commented Dec 27, 2019

Hi @codehearts looks great to me! Thank you!

Copy link
Member

@shadows-withal shadows-withal left a comment

Choose a reason for hiding this comment

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

looks good to me too!

@shadows-withal
Copy link
Member

thank you so much!

@bors: r+

@bors
Copy link
Contributor

bors commented Dec 28, 2019

📌 Commit d25ee55 has been approved by fmoko

@bors
Copy link
Contributor

bors commented Dec 28, 2019

⌛ Testing commit d25ee55 with merge 0d1f1a1...

bors added a commit that referenced this pull request Dec 28, 2019
feat: Show a completion message when watching

The completion message is shown only once all exercises succeed and are
not annotated with "I AM NOT DONE." The watch command will also exit

closes #251

Let me know if there are any tests I could add or if the completion message should be tweaked!
@bors
Copy link
Contributor

bors commented Dec 28, 2019

☀️ Test successful - checks-travis
Approved by: fmoko
Pushing 0d1f1a1 to master...

@bors bors merged commit d25ee55 into rust-lang:master Dec 28, 2019
pedantic79 pushed a commit to pedantic79/rustlings that referenced this pull request Apr 11, 2020
…fmoko

feat: Show a completion message when watching

The completion message is shown only once all exercises succeed and are
not annotated with "I AM NOT DONE." The watch command will also exit

closes rust-lang#251

Let me know if there are any tests I could add or if the completion message should be tweaked!
ppp3 pushed a commit to ppp3/rustlings that referenced this pull request May 23, 2022
…fmoko

feat: Show a completion message when watching

The completion message is shown only once all exercises succeed and are
not annotated with "I AM NOT DONE." The watch command will also exit

closes rust-lang#251

Let me know if there are any tests I could add or if the completion message should be tweaked!
dmoore04 pushed a commit to dmoore04/rustlings that referenced this pull request Sep 11, 2022
…fmoko

feat: Show a completion message when watching

The completion message is shown only once all exercises succeed and are
not annotated with "I AM NOT DONE." The watch command will also exit

closes rust-lang#251

Let me know if there are any tests I could add or if the completion message should be tweaked!
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.

rustlings watch does nothing when all exercises are complete
4 participants