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

run() -> main() #548

Merged
merged 2 commits into from
Sep 19, 2019
Merged

Conversation

workingjubilee
Copy link
Contributor

@workingjubilee workingjubilee commented Sep 19, 2019

Duplicates #441 due to that PR being dead. Fixes #433.
I rebased sepiropht's work into this branch rather than reimplement it myself, and made my own changes to implement the remaining fixes as a separate commit. For this reason, there are two commits in my PR, defying the usual convention for rust-cookbook. It seemed worthwhile to open a new PR including their work since their update is very important for this repo being current and useful.

I did not address gratuitous error-chain usage because that seems like a separate concern. I left a few cases where run() was not being used as an entry point but rather a local function untouched, because it is syntactically valid, even if it is awkward and somewhat dated in style: they can be fixed later.

Checked Before PR Opened:

  • the tests are passing locally with cargo test
  • cookbook renders correctly in mdbook serve -o
  • commits are squashed into two and rebased to latest master
  • PR contains correct "fixes #ISSUE_ID" clause to autoclose the issue on PR merge
  • spell check runs without errors ./ci/spellchecker.sh
  • link check runs without errors link-checker ./book ish
  • non rendered items are in sorted order (links, reference, identifiers, Cargo.toml)
  • links to docs.rs have wildcard version https://docs.rs/tar/*/tar/struct.Entry.html
  • example has standard error handling
  • code identifiers in description are in hyperlinked backticks
[`Entry::unpack`]: https://docs.rs/tar/*/tar/struct.Entry.html#method.unpack

To Check After PR Opens:

  • check if CI is happy with your PR

@workingjubilee workingjubilee changed the title Run to main run() -> main() Sep 19, 2019
@AndyGauge
Copy link
Collaborator

Glad you addressed it, having this old way of doing things in a reference that helps newcomers is a bad idea. I'm going to try to fix the build failure in master and then look to merge this.

@workingjubilee
Copy link
Contributor Author

Both of the issues are on nightly. I believe my other PR fixes the sole failure on the Mac build, and that of the Xenial issues, only one is maybe introduced by this PR. They all seem to deal with the rand crate.

@AndyGauge
Copy link
Collaborator

#542
I think rust skeptic is using rand 0.4 which doesn't have rand::distributions::Distribution when compiled on nightly. I can reproduce it locally, and I'm seeing in cargo tree the only place rand doesn't have Distribution is < 0.5

@AndyGauge
Copy link
Collaborator

That didn't fix it. There is something really strange going on here.

@workingjubilee
Copy link
Contributor Author

I started experimenting a little with things here.

@AndyGauge
Copy link
Collaborator

AndyGauge commented Sep 19, 2019

That didn't fix it. There is something really strange going on here.

I was able to fork rust-skeptic and replace the tempdir with tempfile, so there is no rand < .5 references, did a cargo clean and cargo build and cargo test and still get the behavior.

@AndyGauge
Copy link
Collaborator

Taking postgres out of the cargo.toml file fixes the error lol

@workingjubilee
Copy link
Contributor Author

Ah, I'll revert my last few commits then.

@AndyGauge
Copy link
Collaborator

I'm working on a pr that removes tempdir as well because I think they both need to be nixed.

Thank you so much for looking into this!

@AndyGauge
Copy link
Collaborator

Do you want to resolve the merge conflict in src/web/clients/download/basic.md ?

@workingjubilee
Copy link
Contributor Author

Just did.

@AndyGauge
Copy link
Collaborator

Aaaand I broke master. I'm going to merge this and will work on the failures later today.

@AndyGauge AndyGauge merged commit 5eba1a6 into rust-lang-nursery:master Sep 19, 2019
@workingjubilee
Copy link
Contributor Author

R I P master

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.

Replace quick_main with main returning Result
3 participants