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

Split imports for directory strategy in buf generate #3203

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

oliversun9
Copy link
Contributor

@oliversun9 oliversun9 commented Aug 1, 2024

This is still WIP, but it seems to work with the base case.

Fixes #3056, the problem where only non-imports are split by directory in buf generate. The original issue has a great description and repro.

Note, this cannot be achieved by simply removing the import check here, because the split images will conflate IsImport with whether we want to generate a file if permitted by include-imports. The latter is introduced to a new type ImageFileForGeneration.

TODOs:

  • add more detailed tests
  • cleanup the code
  • there are still places (*_test.go and .../alpha/protoc/plugin.go) that call ImageByDir, they should stop calling it.
  • Now their are functions that call ImageByDir (the current implementation) and those that call ImageByDirSplitImports, perhaps we can completely get rid of ImageByDir.
  • Perhaps move bufimagegenerate into bufgen

This also refactors some generation-specific functions/types into a new bufimagegnerate package, lots of moving around. I will add comments indicating which changes are new code and which ones are relocation.

Copy link
Contributor

github-actions bot commented Aug 1, 2024

The latest Buf updates on your PR. Results from workflow Buf CI / buf (pull_request).

BuildFormatLintBreakingUpdated (UTC)
✅ passed✅ passed✅ passed✅ passedAug 21, 2024, 5:54 PM

@oliversun9 oliversun9 changed the title [WIP] Split imports as well for directory strategy in buf generate [WIP] Split imports for directory strategy in buf generate Aug 1, 2024
@bufdev
Copy link
Member

bufdev commented Aug 1, 2024

@doriable didn't I have a partial PR for this? Is this using that?

@bufdev bufdev changed the title [WIP] Split imports for directory strategy in buf generate Split imports for directory strategy in buf generate Aug 21, 2024
@bufdev bufdev added the WIP label Aug 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

include_imports does not respect 'strategy: directory'
2 participants