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 feature check to doctests #9

Closed
wants to merge 1 commit into from

Conversation

yrns
Copy link
Contributor

@yrns yrns commented Sep 11, 2023

I noticed many doctests fail since JanetClient isn't in the default feature set. The doctests can be wrapped with #[cfg_attr(..., doc = "...")] to skip them with the default features. I can do the rest if you think this is worth the effort (there's a lot). Or, just close if not.

@codecov-commenter
Copy link

Welcome to Codecov 🎉

Once merged to your default branch, Codecov will compare your coverage reports and display the results in this comment.

Thanks for integrating Codecov - We've got you covered ☂️

@GrayJack
Copy link
Owner

It is not as syntactically pleasing as inline docs and we would lose the example when the features are not set, but it can be very useful.

Another two options that passed my mind are:

  1. to apply ignore after the code block initializer ("```") when the features needed for the client are not set or
  2. make the amalgation feature default.

The first one I have no idea how feasible it is to do.

The second will make it more bothersome for users that want to write a native lib for Janet and people using the system Janet, so I'm not sure if this is a good path

So, unless the first is feasible and easy to do, I think this is worth the effort.

I'll do some more digging today how feasible option 1 is

@yrns
Copy link
Contributor Author

yrns commented Sep 12, 2023

Is this better or worse? Not sure. It works, at least.

    #[cfg_attr(any(feature = "amalgation", feature = "link-system"), doc = "```")]
    #[cfg_attr(
        not(any(feature = "amalgation", feature = "link-system")),
        doc = "```ignore"
    )]
2. make the `amalgation` feature default.

Users can always disable default features. Maybe this is best?

@GrayJack
Copy link
Owner

Decided to go with option 2: Done in commit: 4d4a730

@GrayJack GrayJack closed this Nov 20, 2024
@yrns yrns deleted the doctest-feature-check branch November 21, 2024 16:45
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.

3 participants