Skip to content

fix: cargo docs breaking in buildDepsOnly of workspaces#410

Merged
ipetkov merged 3 commits into
ipetkov:masterfrom
dpc:cargo-doc-deps-only-fix
Oct 4, 2023
Merged

fix: cargo docs breaking in buildDepsOnly of workspaces#410
ipetkov merged 3 commits into
ipetkov:masterfrom
dpc:cargo-doc-deps-only-fix

Conversation

@dpc

@dpc dpc commented Oct 2, 2023

Copy link
Copy Markdown
Contributor

Motivation

Fix #312 by using a different name for dummy binary in every package (derived from the package name itself). This should make cargo doc happy. I tested lightly and it seems to work.

Checklist

  • added tests to verify new behavior
  • added an example template or updated an existing one
  • updated docs/API.md (or general documentation) with changes
  • updated CHANGELOG.md

@ipetkov

ipetkov commented Oct 2, 2023

Copy link
Copy Markdown
Owner

Thanks for the PR! Is it possible to add a test for this to avoid future regressions?

@dpc

dpc commented Oct 2, 2023

Copy link
Copy Markdown
Contributor Author

Now that I have some idea how the tests work, I can try adding it later today.

BTw. I don't understand the test failed

@ipetkov

ipetkov commented Oct 2, 2023

Copy link
Copy Markdown
Owner

We have some tests that cleanCargoToml outputs things as expected. Since this changes the name used for dummy binaries those tests might need to be updated as well (I'd recommend building the failing test locally to help with debugging)

@dpc
dpc force-pushed the cargo-doc-deps-only-fix branch from 5082d14 to 7f10f3f Compare October 3, 2023 17:58
@dpc

dpc commented Oct 3, 2023

Copy link
Copy Markdown
Contributor Author

Fixed, added test, made sure test failed before, rebased, pushed. @ipetkov

@ipetkov ipetkov left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thanks for adding the test, LGTM!

@ipetkov
ipetkov merged commit 0da36bc into ipetkov:master Oct 4, 2023
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.

Duplicate crane-dummy after upgrading crane to v0.12.1

2 participants