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.

Sign up for GitHub

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

Jump to bottom

Adding new Fuchsia rustup docs... reworking walkthrough #100927

Merged
merged 3 commits into from
Aug 31, 2022
Merged

Adding new Fuchsia rustup docs... reworking walkthrough #100927

merged 3 commits into from
Aug 31, 2022

Conversation

andrewpollack
Copy link
Member

@andrewpollack andrewpollack commented Aug 23, 2022 •
edited
Loading

Docs improvements:

  • Adding new rustup target add for Fuchsia targets
  • Reworking walkthrough to show directory building as it happens
  • Reworking walkthrough to use hello_fuchsia_pkg/ directory

cc. @djkoloski

@rust-highfive
Copy link
Collaborator

r? @JohnTitor

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 23, 2022
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@andrewpollack
Copy link
Member Author

r? @tmandry

@rust-highfive rust-highfive assigned tmandry and unassigned JohnTitor Aug 23, 2022
@tmandry
Copy link
Member

tmandry commented Aug 23, 2022

@djkoloski want to do a first pass?

djkoloski
djkoloski suggested changes Aug 23, 2022
View reviewed changes
Copy link
Contributor

@djkoloski djkoloski left a comment

Choose a reason for hiding this comment

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

Looks generally good. Specific things we should improve:

  • Make downloading the SDK more obvious if readers are liable to skip to the numbered steps.
  • Consider renaming the directories to make them shorter and clearer. I suggest hello_fuchsia_src, hello_fuchsia_bin, and hello_fuchsia_pkg.
  • Fix up directory structure trees. Looks like some of the subdirs may not be properly laid out and there are some extra legs dangling down.

src/doc/rustc/src/platform-support/fuchsia.md Outdated Show resolved Hide resolved
src/doc/rustc/src/platform-support/fuchsia.md Outdated
Finally, we can build our rust binary as:

```sh
cargo build --release --target x86_64-fuchsia --manifest-path hello_fuchsia_rust_crate/Cargo.toml
Copy link
Contributor

Choose a reason for hiding this comment

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

Is --manifest-path necessary here? Cargo should search for it based on the cwd.

I also think we should default to a debug build unless there's a compelling reason not to.

Copy link
Member Author

Choose a reason for hiding this comment

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

For the walkthrough, I've built all commands to be run from the same base directory. Since cargo new creates the hello_fuchsia_rust_crate/ directory, it can't find the Cargo.toml unless specified

Agreed on debug change!

src/doc/rustc/src/platform-support/fuchsia.md Outdated Show resolved Hide resolved
src/doc/rustc/src/platform-support/fuchsia.md Show resolved Hide resolved
src/doc/rustc/src/platform-support/fuchsia.md Outdated Show resolved Hide resolved
src/doc/rustc/src/platform-support/fuchsia.md Outdated Show resolved Hide resolved
src/doc/rustc/src/platform-support/fuchsia.md Outdated Show resolved Hide resolved
src/doc/rustc/src/platform-support/fuchsia.md Show resolved Hide resolved
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@andrewpollack andrewpollack force-pushed the fuchsia-docs-rustup branch 3 times, most recently from 33e75a5 to 0f87a21 Compare August 23, 2022 22:39
@andrewpollack andrewpollack force-pushed the fuchsia-docs-rustup branch 3 times, most recently from 8920f11 to 70ee3df Compare August 23, 2022 23:12
djkoloski
djkoloski reviewed Aug 24, 2022
View reviewed changes
src/doc/rustc/src/platform-support/fuchsia.md Outdated Show resolved Hide resolved
src/doc/rustc/src/platform-support/fuchsia.md Outdated Show resolved Hide resolved
src/doc/rustc/src/platform-support/fuchsia.md Outdated Show resolved Hide resolved
src/doc/rustc/src/platform-support/fuchsia.md Outdated Show resolved Hide resolved
src/doc/rustc/src/platform-support/fuchsia.md Outdated Show resolved Hide resolved
src/doc/rustc/src/platform-support/fuchsia.md Outdated Show resolved Hide resolved
@andrewpollack
Copy link
Member Author

Stare at the directory artwork for too long, and it will stare right back 😨

Thanks for the feedback @djkoloski & @tmandry !!

djkoloski
djkoloski approved these changes Aug 26, 2022
View reviewed changes
Copy link
Contributor

@djkoloski djkoloski left a comment

Choose a reason for hiding this comment

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

LGTM, one nit that's optional to address

src/doc/rustc/src/platform-support/fuchsia.md Outdated Show resolved Hide resolved
@andrewpollack andrewpollack requested review from djkoloski and tmandry and removed request for djkoloski August 26, 2022 17:20
@djkoloski
Copy link
Contributor

@tmandry This looks good to me

@tmandry
Copy link
Member

tmandry commented Aug 30, 2022

Left a few minor comments, otherwise this looks good to me! Comment @bors r=tmandry once everything's addressed.

@rustbot author
@bors rollup delegate+

@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 30, 2022
@bors

This comment was marked as outdated.

Sign in to view
@rustbot rustbot added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Aug 30, 2022
@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 30, 2022
@tmandry
Copy link
Member

tmandry commented Aug 30, 2022

@bors r- rollup delegate+

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 30, 2022
@bors
Copy link
Contributor

bors commented Aug 30, 2022

✌️ @andrewpollack can now approve this pull request

@andrewpollack
Copy link
Member Author

@bors r=tmandry

@bors
Copy link
Contributor

bors commented Aug 30, 2022

📌 Commit c52d99f has been approved by tmandry

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 30, 2022
@JohnTitor JohnTitor mentioned this pull request Aug 30, 2022
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 31, 2022
@bors
Auto merge of rust-lang#101220 - JohnTitor:rollup-ov7upr7, r=JohnTitor
7f442f8 
Rollup of 10 pull requests

Successful merges:

 - rust-lang#100804 (Fix search results color on hover for ayu theme)
 - rust-lang#100892 (Add `AsFd` implementations for stdio types on WASI.)
 - rust-lang#100927 (Adding new Fuchsia rustup docs... reworking walkthrough)
 - rust-lang#101088 (Set DebuginfoKind::Pdb in msvc_base)
 - rust-lang#101159 (add tracking issue number to const_slice_split_at_not_mut)
 - rust-lang#101192 (Remove path string)
 - rust-lang#101193 (Avoid zeroing large stack buffers in stdio on Windows)
 - rust-lang#101197 (:arrow_up: rust-analyzer)
 - rust-lang#101200 (Add test for issue rust-lang#85872)
 - rust-lang#101219 (Update books)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 3de558d into rust-lang:master Aug 31, 2022
@rustbot rustbot added this to the 1.65.0 milestone Aug 31, 2022
@andrewpollack andrewpollack mentioned this pull request Aug 31, 2022
Merged
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Sep 1, 2022
@Dylan-DPC
Rollup merge of rust-lang#101256 - andrewpollack:fuchsia-docs-adding,…
af32194 
… r=tmandry

Fixes/adjustments to Fuchsia doc walkthrough

Small fixes/adjustments missed during rust-lang#100927
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Sep 1, 2022
@Dylan-DPC
Rollup merge of rust-lang#101256 - andrewpollack:fuchsia-docs-adding,…
291be63 
… r=tmandry

Fixes/adjustments to Fuchsia doc walkthrough

Small fixes/adjustments missed during rust-lang#100927
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Sep 1, 2022
@Dylan-DPC
Rollup merge of rust-lang#101256 - andrewpollack:fuchsia-docs-adding,…
0472344 
… r=tmandry

Fixes/adjustments to Fuchsia doc walkthrough

Small fixes/adjustments missed during rust-lang#100927
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Sep 1, 2022
@Dylan-DPC
Rollup merge of rust-lang#101256 - andrewpollack:fuchsia-docs-adding,…
8c9564e 
… r=tmandry

Fixes/adjustments to Fuchsia doc walkthrough

Small fixes/adjustments missed during rust-lang#100927
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Sep 1, 2022
@matthiaskrgr
Rollup merge of rust-lang#101256 - andrewpollack:fuchsia-docs-adding,…
1cb8ff0 
… r=tmandry

Fixes/adjustments to Fuchsia doc walkthrough

Small fixes/adjustments missed during rust-lang#100927
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Sep 1, 2022
@matthiaskrgr
Rollup merge of rust-lang#101256 - andrewpollack:fuchsia-docs-adding,…
2dfa318 
… r=tmandry

Fixes/adjustments to Fuchsia doc walkthrough

Small fixes/adjustments missed during rust-lang#100927
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Sep 1, 2022
@matthiaskrgr
Rollup merge of rust-lang#101256 - andrewpollack:fuchsia-docs-adding,…
cbcea24 
… r=tmandry

Fixes/adjustments to Fuchsia doc walkthrough

Small fixes/adjustments missed during rust-lang#100927
@tmandry tmandry added the O-fuchsia Operating system: Fuchsia label May 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tmandry tmandry tmandry left review comments

@stormbrew stormbrew stormbrew left review comments

@djkoloski djkoloski djkoloski approved these changes

Assignees

@tmandry tmandry

Labels
O-fuchsia Operating system: Fuchsia S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
1.65.0
Development

Successfully merging this pull request may close these issues.
9 participants
@andrewpollack @rust-highfive @rust-log-analyzer @tmandry @djkoloski @bors @stormbrew @JohnTitor @rustbot