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

Update README.md #496

Merged
merged 15 commits into from
Jan 6, 2020
Merged

Update README.md #496

merged 15 commits into from
Jan 6, 2020

Conversation

davidbarsky
Copy link
Member

Add a quick "getting started" guide for tracing. I'd like to expand this further, but something is better than nothing.

Partially borrowed from rust-lang/log's README.md.

@davidbarsky davidbarsky requested a review from a team December 30, 2019 19:20
hawkw
hawkw reviewed Dec 30, 2019
View reviewed changes
Copy link
Member

@hawkw hawkw left a comment

Choose a reason for hiding this comment

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

This looks good to me. However, I wonder if this should (also?) be in the tracing/README.md file, since that shows up in the tracing crate's crates.io page...

hawkw
hawkw reviewed Dec 30, 2019
View reviewed changes
Copy link
Member

@hawkw hawkw left a comment

Choose a reason for hiding this comment

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

some nits about the examples...i'm not convinced using targets in the basic "getting started" examples is a good idea, maybe we should just show the simplest usages...

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@davidbarsky
Copy link
Member Author

Thanks for the review. I copied this example from the fmt directory examples, but I can reduce this to be a bit more accessible and understandable.

@hawkw
Copy link
Member

hawkw commented Dec 31, 2019

@davidbarsky i think the difference between these examples and the examples dir ones is that the examples dir was intended to demonstrate all the features of the crate, while these should demonstrate the most common ones. also the examples dir needs to be updated in a few places...

davidbarsky
davidbarsky commented Dec 31, 2019
View reviewed changes
README.md Outdated Show resolved Hide resolved
@davidbarsky
Copy link
Member Author

I'd also like to include this example in the tracing crate's README.md and maybe it's top-level docs, but I'll add that to this PR in a bit.

@davidbarsky
Copy link
Member Author

@hawkw I've updated this PR as per our discussion.

@davidbarsky davidbarsky requested a review from hawkw January 4, 2020 18:14
hawkw
hawkw reviewed Jan 4, 2020
View reviewed changes
Copy link
Member

@hawkw hawkw 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 getting really close. i commented on few couple nits, and one typo in code.

also, the previous README contained very similar documentation to the tracing/src/lib.rs docs...can we try to keep these in sync where applicable? thanks!

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
davidbarsky and others added 4 commits January 4, 2020 14:08
@davidbarsky @hawkw
Update tracing/README.md
fc2c5bc 
Co-Authored-By: Eliza Weisman <eliza@buoyant.io>
@davidbarsky @hawkw
Update README.md
44a21e2 
Co-Authored-By: Eliza Weisman <eliza@buoyant.io>
@davidbarsky @hawkw
Update README.md
085abf7 
Co-Authored-By: Eliza Weisman <eliza@buoyant.io>
@davidbarsky
more comments + docs
2c47e10
@davidbarsky
Copy link
Member Author

I pushed the requested changes except for:

also, the previous README contained very similar documentation to the tracing/src/lib.rs docs...can we try to keep these in sync where applicable? thanks!

If my changes to the READMEs look good to you, I'll copy it over to tracing/tracing/lib.rs.

@davidbarsky davidbarsky requested a review from hawkw January 4, 2020 20:32
@hawkw
Copy link
Member

hawkw commented Jan 4, 2020

I pushed the requested changes except for:

also, the previous README contained very similar documentation to the tracing/src/lib.rs docs...can we try to keep these in sync where applicable? thanks!

If my changes to the READMEs look good to you, I'll copy it over to tracing/tracing/lib.rs.

oh, whoops, i missed that. sorry, my bad.

@davidbarsky
Copy link
Member Author

oh, whoops, i missed that. sorry, my bad.

@hawkw That was after your initial round of review :)

hawkw
hawkw approved these changes Jan 4, 2020
View reviewed changes
Copy link
Member

@hawkw hawkw left a comment

Choose a reason for hiding this comment

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

some more nits, but feel free to merge when you feel good about it

README.md Outdated
}
```

In `src/yak_shave.rs`:
Copy link
Member

Choose a reason for hiding this comment

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

TIOLI: I don't know if it's really necessary to show multiple files in the example...I split the examples/ dir version just so that some of the code could be used in multiple examples...

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah, removed that stuff.

README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
davidbarsky and others added 4 commits January 4, 2020 16:42
@davidbarsky @hawkw
Update README.md
f1d4d2b 
Co-Authored-By: Eliza Weisman <eliza@buoyant.io>
@davidbarsky @hawkw
Update README.md
84df442 
Co-Authored-By: Eliza Weisman <eliza@buoyant.io>
@davidbarsky
review comments
ffa58d0
@davidbarsky
Merge remote-tracking branch 'origin/master' into david/update-main-r…
7e02e2e 
…eadme
@davidbarsky davidbarsky merged commit 1dfb657 into master Jan 6, 2020
@davidbarsky davidbarsky deleted the david/update-main-readme branch January 6, 2020 16:16
@hawkw
Copy link
Member

hawkw commented Jan 8, 2020

@davidbarsky is there still going to be a follow-up PR to get these changes reflected in the RustDoc as well? I'd like to get that into the next tracing release if possible.

@hawkw hawkw mentioned this pull request Jan 8, 2020
Closed
@davidbarsky
Copy link
Member Author

@hawkw I've started movement into the RustDoc, but I didn't complete that yet. See here: https://tracing.rs/tracing/#for-log-users. So yes, it might be enough for another release.

@davidbarsky davidbarsky mentioned this pull request Jan 8, 2020
Open
@hawkw hawkw mentioned this pull request Jan 9, 2020
Closed
hawkw added a commit that referenced this pull request Jan 10, 2020
@hawkw
tracing: prepare to release 0.1.12
e7eccb8 
Added

- `Span::with_subscriber` method to access the subscriber that tracks a
  `Span` (#503)
- API documentation now shows which features are required by
  feature-flagged items (#523)
- Improved README examples (#496)
- Documentation links to related crates (#507)

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
@hawkw hawkw mentioned this pull request Jan 10, 2020
Merged
hawkw added a commit that referenced this pull request Jan 11, 2020
@hawkw
tracing: prepare to release 0.1.12
1dca33d 
Added

- `Span::with_subscriber` method to access the subscriber that tracks a
  `Span` (#503)
- API documentation now shows which features are required by
  feature-flagged items (#523)
- Improved README examples (#496)
- Documentation links to related crates (#507)

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
hawkw added a commit that referenced this pull request Jan 11, 2020
@hawkw
tracing: prepare to release 0.1.12 (#530)
6fcdf94 
* tracing: update core version

Signed-off-by: Eliza Weisman <eliza@buoyant.io>

* tracing: prepare to release 0.1.12

Added

- `Span::with_subscriber` method to access the subscriber that tracks a
  `Span` (#503)
- API documentation now shows which features are required by
  feature-flagged items (#523)
- Improved README examples (#496)
- Documentation links to related crates (#507)

Signed-off-by: Eliza Weisman <eliza@buoyant.io>

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

@hawkw hawkw hawkw 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
@davidbarsky @hawkw