Skip to content

issue_130_stabilization_guide #262

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 15 commits into from
Jan 18, 2019
Merged

issue_130_stabilization_guide #262

merged 15 commits into from
Jan 18, 2019

Conversation

rajcspsg
Copy link
Contributor

@mark-i-m @pietroalbini This PR addresses [Stabilization guide] of issue #133.

Please review and let me know your feedback.

@mark-i-m mark-i-m mentioned this pull request Jan 16, 2019
Closed
9 tasks
mark-i-m
mark-i-m suggested changes Jan 16, 2019
View reviewed changes
Copy link
Member

@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.

Thanks @rajcspsg! This is a much needed chapter 👍

Overall, it looks good, but I left a few nits.

Also, could you replace the TODO at the end of the walkthrough with a reference to your new chapter?

@mark-i-m mark-i-m mentioned this pull request Jan 16, 2019
Closed
@mark-i-m
Copy link
Member

mark-i-m commented Jan 16, 2019 •
edited
Loading

Can you also file PR on the forge repo to remove the stabilization guide from there?

@rajcspsg rajcspsg force-pushed the issue_130_stabilization branch 4 times, most recently from be6d700 to b911da2 Compare January 17, 2019 06:02
@rajcspsg
Copy link
Contributor Author

rajcspsg commented Jan 17, 2019 •
edited
Loading

@mark-i-m I get error cannot find value xyz in this scope in places where I have the code in below format -

```rust
// some rust code
// ```

Please see below -
https://travis-ci.com/rust-lang/rustc-guide/builds/97640261

It looks like rust tries to compiles the section of code. I tried lot of ways not sure how to fix this.
Any idea?

@mark-i-m
Copy link
Member

Ah, sorry. I always forget about that. Change the it to ```rust,ignore. This will give us syntax highlighting without trying to compile the code snippet.

@mark-i-m
Copy link
Member

Also, I'm not sure if you saw them, but it looks like Github collapsed a few comments.

@rajcspsg
Copy link
Contributor Author

Thanks @mark-i-m Yeah I'm aware of it. comments got collapsed after I merged your commit suggestions.

@mark-i-m
Copy link
Member

I meant that Github also hid 8 more comments above:

image

@rajcspsg rajcspsg force-pushed the issue_130_stabilization branch from b911da2 to a27b5a5 Compare January 17, 2019 19:00
mark-i-m
mark-i-m approved these changes Jan 17, 2019
View reviewed changes
Copy link
Member

@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.

Looks good to me! I found a few minor typos, but after that, r=me!

Thanks you! This is a great addition.

src/stabilization_guide.md Outdated
## Documentation PRs

If any documentation for this feature exists, it should be
in the `Unstable Book`, located at `src/doc/unstable-book`.
Copy link
Member

Choose a reason for hiding this comment

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

Could you make these into links to https://doc.rust-lang.org/unstable-book/index.html and https://github.com/rust-lang/rust/tree/master/src/doc/unstable-book, respectively?

@rajcspsg rajcspsg force-pushed the issue_130_stabilization branch from e666740 to 1cdf499 Compare January 17, 2019 22:10
@rajcspsg rajcspsg force-pushed the issue_130_stabilization branch 2 times, most recently from 6340486 to 65856b4 Compare January 17, 2019 22:50
@rajcspsg rajcspsg force-pushed the issue_130_stabilization branch from 65856b4 to c110fab Compare January 17, 2019 22:55
@rajcspsg
Copy link
Contributor Author

@mark-i-m is it possible to rebuild this https://travis-ci.com/rust-lang/rustc-guide/builds/97764368?

@mark-i-m
Copy link
Member

Sure I just restarted that build.

@rajcspsg
Copy link
Contributor Author

Thanks @mark-i-m I've incorporated your comment. Please review and let me know if any changes are required.

@mark-i-m mark-i-m merged commit 72237ed into rust-lang:master Jan 18, 2019
@mark-i-m
Copy link
Member

Thanks!

@rajcspsg
Copy link
Contributor Author

@mark-i-m do I need to send PR to rust forge to remove stabilization comment?

@mark-i-m
Copy link
Member

Already done 😄 rust-lang/rust-forge#189

@rajcspsg
Copy link
Contributor Author

Thanks @mark-i-m :)

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

@mark-i-m mark-i-m mark-i-m 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.
2 participants
@rajcspsg @mark-i-m