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.

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

Improve docs & codecov for 0.5.0 #56

Merged
merged 57 commits into from
Jul 9, 2024
Merged

Improve docs & codecov for 0.5.0 #56

merged 57 commits into from
Jul 9, 2024

Conversation

chanced
Copy link
Owner

@chanced chanced commented Jul 2, 2024

  • improves documentation
  • brings code coverage to 99%
  • fixes TryFrom for Index
  • adds FromStr impl for Index

@codecov-commenter
Copy link

codecov-commenter commented Jul 2, 2024

Codecov Report

Attention: Patch coverage is 99.89529% with 1 line in your changes missing coverage. Please review.

Project coverage is 99.1%. Comparing base (a67948d) to head (b90b5e3).

Additional details and impacted files
Files Coverage Δ
src/assign.rs 99.6% <100.0%> (+3.4%) ⬆️
src/component.rs 100.0% <100.0%> (ø)
src/delete.rs 99.4% <100.0%> (+1.7%) ⬆️
src/index.rs 100.0% <100.0%> (+61.9%) ⬆️
src/resolve.rs 100.0% <100.0%> (+10.8%) ⬆️
src/token.rs 100.0% <100.0%> (+22.2%) ⬆️
src/pointer.rs 98.1% <99.7%> (+20.6%) ⬆️

... and 1 file with indirect coverage changes

@chanced chanced marked this pull request as ready for review July 7, 2024 22:01
@chanced
Copy link
Owner Author

chanced commented Jul 8, 2024

@asmello this is ready for you to review whenever you have time.

@chanced
Copy link
Owner Author

chanced commented Jul 8, 2024

Hmm, looking at the README, I'm kinda second guessing not using the strategy you linked.

@chanced
Copy link
Owner Author

chanced commented Jul 8, 2024

I went ahead and employed the rustdoc hack.

Copy link
Collaborator

@asmello asmello left a comment

Choose a reason for hiding this comment

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

Awesome docs! I'm a bit sleepy but I don't think I would have missed anything major.

I feel like a few tests are too trivial, which is what I feared was going to happen when trying to reach near 100% coverage. But fine, leave them, it's good marketing I guess. :p

README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
src/delete.rs Show resolved Hide resolved
src/delete.rs Show resolved Hide resolved
src/pointer.rs Outdated Show resolved Hide resolved
src/pointer.rs Outdated Show resolved Hide resolved
assert_eq!(err.source_offset(), 0);

let err = Pointer::parse("no-leading/slash").unwrap_err();
assert!(err.source().is_none());
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure if this should be tested, is adding a source a breaking change? Ditto elsewhere.

Copy link
Owner Author

Choose a reason for hiding this comment

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

It was to get coverage on that branch of source 😅. I don't know if it'd be considered a breaking change but I would assume so?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Created an issue for this one too: #57

src/pointer.rs Outdated Show resolved Hide resolved
src/pointer.rs Show resolved Hide resolved
@chanced
Copy link
Owner Author

chanced commented Jul 8, 2024

I feel like a few tests are too trivial, which is what I feared was going to happen when trying to reach near 100% coverage.

Yea, my silly ass pursuit of 100 definitely resulted in some obnoxious tests. There were a few tests which I genuinely did not want to write. There were a few, mostly around error outputs, that I let copilot spit out. Yet, I pushed ahead thinking I might actually be able to hit 100 on this lib.

As annoying and useless as some of the tests are, one of the tests that I absolutely thought was pointless caught an issue with TryFrom.

I'm okay with dumping the useless ones. You've marked a few of them.

@chanced
Copy link
Owner Author

chanced commented Jul 9, 2024

I created two issues for the last outstanding conversations: #57 and #58

@chanced chanced merged commit 89bfee5 into main Jul 9, 2024
20 checks passed
@chanced chanced deleted the improve-docs-codecov branch July 9, 2024 02:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants