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

Test attributes deserve their own section #382

Merged
merged 3 commits into from
Sep 1, 2018
Merged

Test attributes deserve their own section #382

merged 3 commits into from
Sep 1, 2018

Conversation

Havvy
Copy link
Contributor

@Havvy Havvy commented Aug 1, 2018

cc @QuietMisdreavus and @steveklabnik for possibly out-of-scope documentation here.

@Havvy
Copy link
Contributor Author

Havvy commented Aug 1, 2018 •
edited
Loading

Do not merge both this and the previous PR at the same time until I've edited the other PR to remove the "function-only attributes" section since the two PRs remove all of the attributes from that section.

@Havvy Havvy mentioned this pull request Aug 3, 2018
Merged
steveklabnik
steveklabnik approved these changes Aug 18, 2018
View reviewed changes
Copy link
Member

@steveklabnik steveklabnik left a comment

Choose a reason for hiding this comment

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

LGTM :)

@Havvy
Copy link
Contributor Author

Havvy commented Aug 19, 2018

With the other PR merged, this one now also handles removing the entirety of the "Function only items" section and can be merged.

Hmm, I probably should have done that in a new commit, but it's 2am and I'm not thinking straight. But wanted to get this unblocked before I head to bed. Sorry. The only change since the previous comment is just the removal of that header.

@Havvy Havvy added the New Content Missing features or aspects of language not currently documented. label Aug 20, 2018
@Havvy Havvy requested a review from matthewjasper September 1, 2018 08:02
matthewjasper
matthewjasper approved these changes Sep 1, 2018
View reviewed changes
Copy link
Contributor

@matthewjasper matthewjasper left a comment

Choose a reason for hiding this comment

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

One nit, then this is good to merge.

src/attributes.md Outdated
> Note: The test harness is ran by passing the `--test` argument to `rustc` or
> using `cargo test`.

Tests that return `()` pass as long as they terminate do not panic. Tests that
Copy link
Contributor

Choose a reason for hiding this comment

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

missing 'and' after 'terminate'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

@matthewjasper matthewjasper merged commit 65e9b3d into rust-lang:master Sep 1, 2018
@matthewjasper
Copy link
Contributor

Thanks!

@Havvy Havvy deleted the test-attr branch September 1, 2018 20:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@steveklabnik steveklabnik steveklabnik approved these changes

@matthewjasper matthewjasper matthewjasper approved these changes

Assignees
No one assigned
Labels
New Content Missing features or aspects of language not currently documented.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Havvy @matthewjasper @steveklabnik