Closed JuanMarchetto closed 3 years ago
Merging #226 (bf3e244) into master (3469f15) will not change coverage. The diff coverage is
n/a
.
@@ Coverage Diff @@
## master #226 +/- ##
=======================================
Coverage 61.17% 61.17%
=======================================
Files 40 40
Lines 2697 2697
=======================================
Hits 1650 1650
Misses 1047 1047
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 3469f15...bf3e244. Read the comment docs.
This is a very cool idea @JuanMarchetto! Great work! π π
I'm glad you chose to submit this early, as big examples like this often need a lot of feedback, so discussing it earlier is always better than later. Lots of code to review!
I won't have a chance to look at this in detail for a few days, but here's a couple of things I noticed from skimming through the code:
letters
instead of write
? It's possible we'll add a write
method in the future (#22) and I won't want this to conflict with the examples we'll eventually add for thatrustfmt
on the files you're adding so the formatting is consistent throughout them
cargo fmt
yet, so please use the rustfmt
tool on your individual files only:
rustfmt examples/letters/*
letters.rs
is going to get pretty big with 26+ letters, it's a good idea to split it up further into modules like a_to_d.rs
, e_to_h.rs
, etc.other_chars.rs
, the name punctuation.rs
might fit better for what's there nowΒ‘
character! I think that's great! π char
is the name of a type in Rust, it's a good idea to avoid using it as a variable name. Usually people go with ch
or character
or something.src/write/main.rs
(which you'll rename to src/letters/main.rs
), please add a short comment (example) that describes what this example does and how to run it. This will help people understand what this code does without having to read all of it.--help
. Since you're learning Rust, I figured this would be a good example of a nice way to write this code using pattern matching and iterators. For actual production applications people usually use structopt (no need for that here). Please incorporate this into your example when you have a chance. π Overall, really great work with this PR! It might take a little longer than last time to review and get merged since there is much more code, but I'm really looking forward to working with you to get this finished. Thanks for taking the time to work on this! π₯³ π
Hey @JuanMarchetto I saw you pushed a new commit. Let me know when you're ready for a review. If you think this PR is ready, you can convert it from a draft PR to a regular PR.
Hey @JuanMarchetto I saw you pushed a new commit. Let me know when you're ready for a review. If you think this PR is ready, you can convert it from a draft PR to a regular PR.
Hey @sunjay this is not ready yet, I've been busy this week, but the next one I'll will get back to this and let you know when is ready. Thanks!
Sounds good! Take as much time as you need. Just wanted to make sure you weren't waiting on me. :)
Happy to help out with anything if you run into any issues. Thanks for all your work!
@sunjay i made some progress here, i don't know how to address the coverage failing status do. and if you what this is ready to rewiev. thanks!
@JuanMarchetto Great! I will have a look over a few days. If I don't get back to you by next week please don't hesitate to ping me. (Hoping to have it done much sooner than that, but thought I'd mention it just in case.)
Don't worry about the coverage status. Only the required statuses + an approval from me are needed to get this merged. :)
@JuanMarchetto Great! I will have a look over a few days. If I don't get back to you by next week please don't hesitate to ping me. (Hoping to have it done much sooner than that, but thought I'd mention it just in case.)
Don't worry about the coverage status. Only the required statuses + an approval from me are needed to get this merged. :)
Hi @sunjay, can you have a look to this PR? Thanks!
Hi @sunjay, can you have a look to this PR? Thanks!
Hey @JuanMarchetto! Sorry for the delay. I saw that you were still pushing things and assumed that you were still working on it. I will take a look over the next few days. Thanks for all your effort! π π
Hey! Really sorry for not getting to your PR yet. Going through some things and will try to find time soon. I usually try to be really quick with these things, but it unfortunately didn't work out this time. I will get to this! :)
Hey! Really sorry for not getting to your PR yet. Going through some things and will try to find time soon. I usually try to be really quick with these things, but it unfortunately didn't work out this time. I will get to this! :)
Hey sunjay, don't worry, i hope you are fine!
Hi, i'm creating this example that draw any text you want. For example, if you run in console:
cargo run --example write hello
it will draw "Hello" if you don't introduce any text:cargo run --example write
it will draw "Hello, World!"you can also pass a font size as a second parameter:
cargo run --example write "Hello, World!" 50
and all the letters will ajust to that size, if you don't pass that second parameter 20.0 will be the default.I still need to implement some more letter but i like to show you the progress at this point. At this point if you introduce a character that currently doesn't have an implemtations will we replaced with '?' and in the console it will trow the followinig message:
I hope you like it.