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

NOISSUE - Create lib.rs, to allow documentation tests to be written… #153

Merged
merged 1 commit into from
Dec 6, 2020

Conversation

jmcconnell26
Copy link
Contributor

… and

run:

  • Add high level description of public modules in lib.rs
  • Add docs and doc-tests for args module
  • Add clippy level to enforce use of doc markdown

Signed-off-by: joshmc [email protected]

@jmcconnell26
Copy link
Contributor Author

Adding this, as I'd like to start trying to add more docs/doc-tests for the code changes in ISSUE-16

@anderejd
Copy link
Contributor

anderejd commented Nov 29, 2020

This change seems to make much of the internals of cargo-geiger public through the pub mod statements in the added lib.rs. Are we not going to have a hard time upholding the stability of this public API over time having exposed so much of the internals?

@jmcconnell26 jmcconnell26 force-pushed the NOISSUE-CreateLibRs branch 2 times, most recently from 837f678 to 2a7d2bb Compare November 29, 2020 17:49
@jmcconnell26
Copy link
Contributor Author

Apologies @anderejd, this is a misunderstanding on my end, I hadn't realised a lib.rs in the same crate as a bin is published.
I had thought it could only be used in the bin itself.
I know there is the pub crate functionality which could solve this problem, but I don't think that applies to modules.
If creating a lib.rs publishes the modules, then you're definitely right, we don't want to expose such a wide range of the internals.
I'm more than happy to close this PR, and plan a more structured approach to getting the doc-tests running.
Thanks!
Josh

@tarcieri
Copy link
Collaborator

You can just slap a warning on it that the API provided by the [[lib]] crate doesn't follow semver until you figure out a way to stabilize the internals that makes sense.

In the meantime people may be interested in experimented with consuming cargo-geiger as a library, which this would also enable. Perhaps interested downstream consumers (cargo-deny comes to mind) could help you figure out what parts of the API to stabilized.

@anderejd
Copy link
Contributor

You can just slap a warning on it that the API provided by the [[lib]] crate doesn't follow semver until you figure out a way to stabilize the internals that makes sense.

In the meantime people may be interested in experimented with consuming cargo-geiger as a library, which this would also enable. Perhaps interested downstream consumers (cargo-deny comes to mind) could help you figure out what parts of the API to stabilized.

@tarcieri's suggestion seems reasonable to me, @jmcconnell26 would you like to use that approach?

@jmcconnell26
Copy link
Contributor Author

@anderejd, @tarcieri, that sounds good to me. I have updated the PR to add the disclaimer, do you think that the wording looks OK?
Thanks,
Josh

@anderejd
Copy link
Contributor

anderejd commented Dec 4, 2020

LGTM. I also think we should add the same or a similar disclaimer somewhere in the README.md.

@jmcconnell26
Copy link
Contributor Author

@anderejd, that sounds like a good idea to me! I'll add a section to the README.md.
Would you object to me pulling out the change log to its own CHANGELOG.md at the same time?

Apologies for the deluge of PRs B.T.W!
Thanks,
Josh

@anderejd
Copy link
Contributor

anderejd commented Dec 4, 2020

Would you object to me pulling out the change log to its own CHANGELOG.md at the same time?

Sure, the README.md is getting a bit long. This will deny users to read the changelog on creates.io though. A link to the changelog from the README.md could be nice, if that works on crates.io (?).

@anderejd
Copy link
Contributor

anderejd commented Dec 4, 2020

Apologies for the deluge of PRs B.T.W!

Keep 'em coming 🚀

… and

run:
* Add high level description of public modules in `lib.rs`
* Add docs and doc-tests for args module
* Add clippy level to enforce use of doc markdown
* Add disclaimer that lib is not stable
* Update README to include disclaimer
* Pull changelog to its own file
* Add link to new CHANGELOG.md file to expose the change log on
  crates.io

Signed-off-by: joshmc <[email protected]>
@jmcconnell26
Copy link
Contributor Author

Sure, the README.md is getting a bit long. This will deny users to read the changelog on creates.io though. A link to the changelog from the README.md could be nice, if that works on crates.io (?).

That's a good shout, I've updated the README.md to add a link to the new file.
I had a quick look on crates.io at the existing links in the same style, and they look to work as expected.

Thanks,
Josh

@anderejd
Copy link
Contributor

anderejd commented Dec 6, 2020

Looks great, thanks! Let's do this.

@anderejd anderejd merged commit bd50a67 into geiger-rs:master Dec 6, 2020
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