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

fix(cargo-workspaces): expand globs in crate paths #1852

Merged
merged 4 commits into from
Mar 13, 2023
Merged

fix(cargo-workspaces): expand globs in crate paths #1852

merged 4 commits into from
Mar 13, 2023

Conversation

Stonks3141
Copy link
Contributor

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

Fixes #1851 🦕

@Stonks3141 Stonks3141 requested review from a team as code owners February 13, 2023 17:34
@product-auto-label product-auto-label bot added the size: m Pull request size is medium. label Feb 13, 2023
Copy link
Contributor

@chingor13 chingor13 left a comment

Choose a reason for hiding this comment

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

I don't think this is going to work like you think. In release-please, there is no cloned repo so a file glob won't actually look at files on disk. Instead, you will probably want to use our implementation of findFilesByName on the GitHub client instance

@Stonks3141
Copy link
Contributor Author

That should be fixed now, the github client has a findFilesByGlob method.

Copy link
Contributor

@chingor13 chingor13 left a comment

Choose a reason for hiding this comment

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

Thanks for this, changes look ok, but the tests are failing because it's making a new API call that isn't mocked. We use sinon to mock out methods. Example:

sandbox.stub(github, 'findFilesByGlob').resolves(['path/to/file1.txt', 'path/to/file2.txt']);

@Stonks3141
Copy link
Contributor Author

Thanks for the review! I couldn’t figure out how to run the tests locally or on my fork, so I just copied a nearby test and hoped for the best! I should be able to work on it tomorrow.

@Stonks3141
Copy link
Contributor Author

This should hopefully be fixed, I applied your suggestion.

@chingor13 chingor13 merged commit 0179f25 into googleapis:main Mar 13, 2023
@Stonks3141 Stonks3141 deleted the cargo-workspace-glob branch March 15, 2023 03:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size: m Pull request size is medium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Glob paths for Cargo workspaces are unsupported
2 participants