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 buftarget package #2799

Merged
merged 32 commits into from
Mar 6, 2024
Merged

Add buftarget package #2799

merged 32 commits into from
Mar 6, 2024

Conversation

doriable
Copy link
Member

@doriable doriable commented Mar 5, 2024

This PR adds a buftarget package, which provides path targeting information
for a storage bucket:

  • BucketTargeting provides information on a ControllingWorkspace if one was
    found in the bucket, the input path, target paths, and target exclude paths
  • TerminateAtControllingWorkspace has been consolidated to this package

The way the package is used is that when bufctl.Controller calls buffetch/internal.Reader
to get the storage bucket , buffetch/internal.Reader now also returns buftarget.BucketTargeting
alongside the bucket. bufworkspace.WorkspaceProvider then handles the workspace and module
targeting using buftarget.BucketTargeting. This removes the need to set input path, target paths,
and target exclude paths as workspace provider options, and is instead just passed through
workspaceTargeting and moduleTargeting.

This change also introduces some fallback logic in workspaceTargeting for inputs that did not
resolve to a controlling workspace.

Lastly, tests that were skipped previously for path/exclude-path behavior issues have been
unskipped and resolved accordingly.

@doriable doriable requested a review from bufdev March 5, 2024 20:35
@bufdev bufdev changed the title Bsr 3326 target paths Add buftarget package Mar 5, 2024
Copy link
Member

@bufdev bufdev left a comment

Choose a reason for hiding this comment

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

Approving to get this into bufmod as this is clearly an improvement over the status quo - we're going to do a line-by-line review as we go, but I've QA'ed the logic and done a basic pass, and we're in a much better state after this.

@bufdev bufdev merged commit a39aac7 into bufmod Mar 6, 2024
7 checks passed
@bufdev bufdev deleted the BSR-3326-target-paths branch March 6, 2024 00:52
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.

2 participants