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

Add code examples for boa_interner #2337

Open
jedel1043 opened this issue Oct 10, 2022 · 12 comments · Fixed by #3141
Open

Add code examples for boa_interner #2337

jedel1043 opened this issue Oct 10, 2022 · 12 comments · Fixed by #3141
Assignees
Labels
API documentation update documentation E-Easy Easy good first issue Good for newcomers
Milestone

Comments

@jedel1043
Copy link
Member

The boa_interner crate has an allow(rustdoc::missing_doc_code_examples) on its root. Ideally, this crate should have some usage snippets on every pub function.

@jedel1043 jedel1043 added good first issue Good for newcomers E-Easy Easy documentation update documentation API labels Oct 10, 2022
@jedel1043 jedel1043 added this to the v0.17.0 milestone Oct 10, 2022
@Joymfl
Copy link

Joymfl commented Oct 18, 2022

Can I pick this up?

@jedel1043
Copy link
Member Author

Can I pick this up?

Assigned!

@Joymfl
Copy link

Joymfl commented Oct 18, 2022

Hey, currently I'm looking at how the pub functions are being used in the boa_interner/src/tests.rs and then documenting code examples for the same, but I see more functions that aren't being used in the tests. Any tips on how to understand and document them?

@jedel1043
Copy link
Member Author

Most of the public functions have usages inside boa_interner itself, so you could start from that. I think the only unused functions are Interner::new and Interner::with_capacity, for which you can adapt the examples of Vec::new and Vec::with_capacity.

@Joymfl
Copy link

Joymfl commented Oct 18, 2022

Sounds good 👍🏾 . I'll try that and get back to you

@postmeback
Copy link
Contributor

@Joymfl , any update on this ?

@Razican Razican modified the milestones: v0.17.0, v0.18.0 Jul 8, 2023
@Razican
Copy link
Member

Razican commented Aug 6, 2023

I don't think this issue should be closed, since the related PR added a couple of tests, but no examples in the documentation of the interner.

@jedel1043 jedel1043 reopened this Aug 6, 2023
@postmeback
Copy link
Contributor

postmeback commented Aug 7, 2023

I don't think this issue should be closed, since the related PR added a couple of tests, but no examples in the documentation of the interner.

Can you give me an reference, where should I add the documentation ?

@jedel1043 @Razican

@postmeback
Copy link
Contributor

Hi, Can i get any assistance on the above comment, please?

@Razican
Copy link
Member

Razican commented Oct 3, 2023

Hi @postmeback, sorry for not writing here!

I was checking the code, and in fact, the mentioned lint is unstable (rust-lang/rust#101730). You could in theory enable it by adding #![feature(rustdoc_missing_doc_code_examples)] and #![warn(rustdoc::missing_doc_code_examples)] in lib.rs in boa_interner, and then run cargo +nightly doc to get the relevant warnings.

This can help you get the warnings on where some usage examples need to be added in the API. boa_interner was selected for this since its public API is small enough.

For each warning, the idea is to add some code examples in the doc comments, that show how to use it.

@postmeback
Copy link
Contributor

I followed your steps, I did not get any warnings in the boa_interner folder.

image

Requesting you to provide your feedback on this. :)

@jedel1043
Copy link
Member Author

I followed your steps, I did not get any warnings in the boa_interner folder.

Saw that you ran cargo +nightly fmt. You would need to run cargo +nightly doc instead to see the warnings.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API documentation update documentation E-Easy Easy good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants