[433] remove old run function #441

sepiropht · 2018-08-12T23:03:40Z

fixes #433

Things to check before submitting a PR

[`Entry::unpack`]: https://docs.rs/tar/*/tar/struct.Entry.html#method.unpack

Things to do after submitting PR

check if CI is happy with your PR

Thank you for reading, you may now delete this text! Thank you! 😄

AndyGauge

I really appreciate this work, but would like to see some additional cleanup as noted in the requests.

AndyGauge · 2018-08-17T23:11:25Z

src/about.md

@@ -107,7 +107,7 @@ Since these recipes are intended to be reused as-is and encourage best
 practices, they set up error handling correctly when there are
 `Result` types involved.

-The basic pattern we use is to have a `fn run() -> Result` that acts
+The basic pattern we use is to have a `fn main() -> Result` that acts


two lines after this, run is referenced again.

AndyGauge · 2018-08-17T23:12:08Z

src/about.md

@@ -167,14 +166,14 @@ use url::{Url, Position};
 #     }
 # }

-fn run() -> Result<()> {
+fn main() -> Result<()> {
    let parsed = Url::parse("https://httpbin.org/cookies/set?k2=v2&k1=v1")?;
    let cleaned: &str = &parsed[..Position::AfterPath];
    println!("cleaned: {}", cleaned);
    Ok(())
 }
 #


These trailing # should be removed too.

sepiropht · 2018-08-18T08:13:58Z

I thought that i have finished but now i have problem with the two build future (see issue #29642) --> C:\Users\appveyor\.cargo\registry\src\github.com-1ecc6299db9ec823\log4rs-0.8.0\src\append\rolling_file\mod.rs:46:1 | 46 | #[serde(deny_unknown_fields)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = help: add #![feature(custom_attribute)] to the crate attributes to enable
This repo is not on discord ?

AndyGauge · 2018-08-20T15:43:37Z

I just tested Master against this, this is a bug introduced by Rust 1.30

I will create a new issue

sepiropht · 2018-08-20T15:47:34Z

Ok thank @AndyGauge !

sepiropht · 2018-08-21T08:19:39Z

We will wait until the bug fix before merge this pr ?

budziq

Thanks @sepiropht
Nice work some minor suggestions follow.
Please double check for any whitespace problems, trailing newlines or unneeded #

budziq · 2018-08-23T13:07:09Z

src/web/url/origin.md

@@ -66,7 +64,7 @@ fn run() -> Result<()> {
    Ok(())
 }
 #


please remove the trailing # and all uses of error_chain when not completely necessary.

budziq · 2018-08-23T13:10:24Z

src/web/clients/api/rate-limited.md

@@ -30,7 +30,7 @@ header! { (XRateLimitLimit, "X-RateLimit-Limit") => [usize] }
 header! { (XRateLimitRemaining, "X-RateLimit-Remaining") => [usize] }
 header! { (XRateLimitReset, "X-RateLimit-Reset") => [u64] }

-fn run() -> Result<()> {
+fn main() -> Result<()> {


This example requires slightly larger rewrite. It was never ideal with recursive run but rename to main has made it even more apparent. Some minimal main should be extracted that calls possibly still recursive run (I'd most likely appreciate rewriting it to iterative form).

budziq · 2018-08-23T13:11:12Z

src/encoding/csv/read.md

@@ -85,7 +83,7 @@ fn run() -> Result<()> {
    Ok(())
 }
 #


please remove this and other occurrences of the trailing #

budziq · 2018-08-23T13:11:23Z

src/encoding/csv/read.md

@@ -85,7 +83,7 @@ fn run() -> Result<()> {
    Ok(())
 }
 #
-# quick_main!(run);
+


no reason for trailing newline

budziq · 2018-08-23T13:11:59Z

src/encoding/complex/toml.md

@@ -92,5 +91,5 @@ fn run() -> Result<()> {
    Ok(())
 }
 #


please remove this and other occurrences of the trailing #

budziq · 2018-08-23T13:12:11Z

src/encoding/complex/toml.md

@@ -37,8 +37,7 @@ fn run() -> Result<()> {

    Ok(())
 }
-#
-# quick_main!(run);
+


no reason for trailing newline

budziq · 2018-08-23T13:12:43Z

src/development_tools/debugging/log/log-syslog.md

@@ -39,11 +39,10 @@ fn run() -> Result<()> {
 # #[cfg(not(target_os = "linux"))]
 # error_chain! {}
 # #[cfg(not(target_os = "linux"))]
-# fn run() -> Result<()> {
+# fn main() -> Result<()> {
 #     Ok(())
 # }
 #


please remove the trailing #

budziq · 2018-08-23T13:14:57Z

src/about.md

-work. The `quick_main!` macro generates the actual `main` function and
-prints out the error if one occurred.
+work. Since rust 1.26, the main function can return a value, no need 
+to use a custom `run` function like before.


This sentence
Since rust 1.26, the main function can return a value, no need to use a custom run function like before. should not be necessary

Thanks for the reviews, i will do this asap

AndyGauge · 2018-09-17T14:59:59Z

I believe nightly rust works again, so I've triggered a build. I approved the commit that happened after my comments, and before Budziq. At this point, there are some more requested changes. Please let me know if you are following the requests, or if you would like fresh eyes on it. Great work so far!

sepiropht · 2018-09-17T17:24:04Z

Ok cool ! I will do all the changes very soon.

sepiropht · 2018-12-07T16:01:41Z

Apart conflicts this branch should be ok no ?

AndyGauge · 2019-01-26T00:23:22Z

I would love to see this land, I'll be working on your branch a little--hope you don't mind.

sepiropht force-pushed the master branch 3 times, most recently from 80e4992 to c62eba1 Compare August 14, 2018 09:53

AndyGauge requested changes Aug 17, 2018

View reviewed changes

sepiropht force-pushed the master branch 2 times, most recently from 89a4f96 to e8d288d Compare August 18, 2018 07:18

budziq requested changes Aug 23, 2018

View reviewed changes

AndyGauge approved these changes Sep 17, 2018

View reviewed changes

sepiropht force-pushed the master branch from e8d288d to 86fc1f8 Compare September 18, 2018 13:13

[433] remove old run function

4351446

sepiropht force-pushed the master branch from 86fc1f8 to 4351446 Compare September 18, 2018 13:20

Merge branch 'master' into master

869fc85

AndyGauge added 3 commits January 25, 2019 16:50

2 type definitions expected

3e371d4

typo

69ee409

Update tar-decompress.md

543e042

workingjubilee mentioned this pull request Sep 19, 2019

run() -> main() #548

Merged

11 tasks

AndyGauge closed this Sep 19, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[433] remove old run function #441

[433] remove old run function #441

sepiropht commented Aug 12, 2018 •

edited

Loading

AndyGauge left a comment

AndyGauge Aug 17, 2018

AndyGauge Aug 17, 2018

sepiropht commented Aug 18, 2018

AndyGauge commented Aug 20, 2018

sepiropht commented Aug 20, 2018

sepiropht commented Aug 21, 2018

budziq left a comment

budziq Aug 23, 2018

budziq Aug 23, 2018

budziq Aug 23, 2018

budziq Aug 23, 2018

budziq Aug 23, 2018

budziq Aug 23, 2018

budziq Aug 23, 2018

budziq Aug 23, 2018

sepiropht Aug 28, 2018

AndyGauge commented Sep 17, 2018

sepiropht commented Sep 17, 2018

sepiropht commented Dec 7, 2018

AndyGauge commented Jan 26, 2019

@@ @@ -66,7 +64,7 @@ fn run() -> Result<()> { @@
                   Ok(())
               }
               #

@@ @@ -85,7 +83,7 @@ fn run() -> Result<()> { @@
                   Ok(())
               }
               #

@@ @@ -92,5 +91,5 @@ fn run() -> Result<()> { @@
                   Ok(())
               }
               #

[433] remove old run function #441

[433] remove old run function #441

Conversation

sepiropht commented Aug 12, 2018 • edited Loading

fixes #433

Things to check before submitting a PR

Things to do after submitting PR

AndyGauge left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sepiropht commented Aug 18, 2018

AndyGauge commented Aug 20, 2018

sepiropht commented Aug 20, 2018

sepiropht commented Aug 21, 2018

budziq left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AndyGauge commented Sep 17, 2018

sepiropht commented Sep 17, 2018

sepiropht commented Dec 7, 2018

AndyGauge commented Jan 26, 2019

sepiropht commented Aug 12, 2018 •

edited

Loading