Skip to content

Overview #633

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
Merged
merged 38 commits into from
Apr 18, 2020
Merged

Overview #633

merged 38 commits into from
Apr 18, 2020

Conversation

mark-i-m
Copy link
Member

@mark-i-m mark-i-m commented Mar 26, 2020 •
edited
Loading

This is our draft so far. Feedback welcome! And thanks to all who contributed 🎉

cc @chrissimpkins @rust-lang/wg-learning

resolves #320

@chrissimpkins chrissimpkins mentioned this pull request Mar 26, 2020
Merged
2 tasks
chrissimpkins
chrissimpkins reviewed Mar 28, 2020
View reviewed changes
@crlf0710

This comment has been minimized.

Sign in to view
ebroto
ebroto reviewed Mar 28, 2020
View reviewed changes
Centril
Centril reviewed Apr 2, 2020
View reviewed changes
@chrissimpkins

This comment has been minimized.

Sign in to view
@mark-i-m

This comment has been minimized.

Sign in to view
@chrissimpkins

This comment has been minimized.

Sign in to view
mark-i-m
mark-i-m commented Apr 2, 2020
View reviewed changes
Copy link
Member Author

@mark-i-m mark-i-m left a comment

Choose a reason for hiding this comment

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

@Centril Thanks for the detailed review. I will return later to address more of your comments.

@mark-i-m

This comment has been minimized.

Sign in to view
@chrissimpkins chrissimpkins mentioned this pull request Apr 3, 2020
Closed
LeSeulArtichaut
LeSeulArtichaut reviewed Apr 3, 2020
View reviewed changes
Copy link
Contributor

@LeSeulArtichaut LeSeulArtichaut left a comment

Choose a reason for hiding this comment

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

Tried to add entry points

@LeSeulArtichaut

This comment has been minimized.

Sign in to view
@mark-i-m

This comment has been minimized.

Sign in to view
@mark-i-m mark-i-m changed the title Overview [WIP] Overview Apr 3, 2020
mark-i-m
mark-i-m commented Apr 4, 2020
View reviewed changes
mark-i-m
mark-i-m commented Apr 4, 2020
View reviewed changes
mark-i-m
mark-i-m commented Apr 4, 2020
View reviewed changes
bjorn3
bjorn3 reviewed Apr 4, 2020
View reviewed changes
bjorn3
bjorn3 reviewed Apr 4, 2020
View reviewed changes
bjorn3
bjorn3 reviewed Apr 4, 2020
View reviewed changes
bjorn3
bjorn3 reviewed Apr 4, 2020
View reviewed changes
@chrissimpkins chrissimpkins mentioned this pull request Apr 6, 2020
Closed
mark-i-m
mark-i-m commented Apr 8, 2020
View reviewed changes
spastorino
spastorino reviewed Apr 13, 2020
View reviewed changes
spastorino
spastorino reviewed Apr 14, 2020
View reviewed changes
spastorino
spastorino reviewed Apr 14, 2020
View reviewed changes
@mark-i-m
Copy link
Member Author

Ok, so as per our discussion in the meeting, we now have a final-ish draft to attempt to merge. Followup work and improvements should be noted on #674. For now, comments and commits on the PR should be limited to fixing incorrect statements.

@mark-i-m mark-i-m changed the title [WIP] Overview Overview Apr 14, 2020
@spastorino
Copy link
Member

spastorino commented Apr 15, 2020 •
edited
Loading

In my opinion this is already good enough.

chrissimpkins
chrissimpkins previously approved these changes Apr 15, 2020
View reviewed changes
Copy link
Member

@chrissimpkins chrissimpkins left a comment

Choose a reason for hiding this comment

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

Some minor edits. Take or leave. Nothing that should block a merge here. Looks great @rust-lang/wg-rustc-dev-guide!

src/overview.md
tremendous amount of change constantly going on.
- Compiler integration: a number of other tools need to use the compiler in
various ways (e.g. cargo, clippy, miri, RLS) that must be supported.
- Compiler stability: the compiler should not crash or fail ungracefully on the
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- Compiler stability: the compiler should not crash or fail ungracefully on the
- Stability: the compiler should not crash or fail ungracefully on the

@mark-i-m @chrissimpkins
Remove various todos
e5f427d 
With some items added to rust-lang#674

Co-Authored-By: Chris Simpkins <[email protected]>
@mark-i-m mark-i-m dismissed stale reviews from chrissimpkins via e5f427d April 18, 2020 02:14
JohnTitor
JohnTitor reviewed Apr 18, 2020
View reviewed changes
JohnTitor
JohnTitor approved these changes Apr 18, 2020
View reviewed changes
Copy link
Member

@JohnTitor JohnTitor left a comment

Choose a reason for hiding this comment

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

Applied some minor nits. Okay, I'd say "go ahead"!

LeSeulArtichaut
LeSeulArtichaut approved these changes Apr 18, 2020
View reviewed changes
Copy link
Contributor

@LeSeulArtichaut LeSeulArtichaut left a comment

Choose a reason for hiding this comment

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

This is looking great! I've got nothing more to say.

src/overview.md
parser, HIR, etc)? - e.g., `cargo rustc -- -Zunpretty=hir-tree` allows you to
view HIR representation
- What is the main source entry point for `X`?
- Where do phases diverge for cross-compilation to machine code across
Copy link
Contributor

Choose a reason for hiding this comment

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

There is a trailing space here 👀

Suggested change
- Where do phases diverge for cross-compilation to machine code across
- Where do phases diverge for cross-compilation to machine code across

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll make a new PR for these

src/overview.md

# Unresolved Questions


Copy link
Contributor

Choose a reason for hiding this comment

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

There are two lines here, unlike the rest of the document.

Suggested change

@mark-i-m mark-i-m merged commit e927db9 into rust-lang:master Apr 18, 2020
@mark-i-m mark-i-m deleted the overview branch April 18, 2020 16:11
@mark-i-m
Copy link
Member Author

🎉 🎉 🎉

@mark-i-m
Copy link
Member Author

Thanks to everyone who was involved!

Followups should go in #674

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@spastorino spastorino spastorino left review comments

@eddyb eddyb eddyb left review comments

@ebroto ebroto ebroto left review comments

@Centril Centril Centril left review comments

@bjorn3 bjorn3 bjorn3 left review comments

@chrissimpkins chrissimpkins chrissimpkins approved these changes

@JohnTitor JohnTitor JohnTitor approved these changes

@LeSeulArtichaut LeSeulArtichaut LeSeulArtichaut approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Write an overview chapter
10 participants
@mark-i-m @crlf0710 @chrissimpkins @LeSeulArtichaut @spastorino @eddyb @ebroto @Centril @bjorn3 @JohnTitor