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

refactor: bazel #280

Merged
merged 14 commits into from
Sep 21, 2022
Merged

refactor: bazel #280

merged 14 commits into from
Sep 21, 2022

Conversation

cgrindel
Copy link
Contributor

@cgrindel cgrindel commented Sep 21, 2022

  • Refactor bazel.bazel
    • Remove workspace.Finder.
    • Add workspaceRoot to store the path to the root of the Bazel workspace.
  • Update bazel.New() to take a workspaceRoot parameter.
  • Add bazel.Find() and bazel.FindFromWd() to find a workspace root and return an initialized Bazel instance.
  • Refactor Bazel interface
    • Remove WithOverrideWorkspaceRoot(). Instances can now be created using a path.
    • Remove Spawn(). Appears to be an alias for RunCommand().
    • Update WithEnv() to not create a new instance when applying the environment variables.

@cgrindel cgrindel marked this pull request as ready for review September 21, 2022 20:59
Copy link
Contributor

@f0rmiga f0rmiga left a comment

Choose a reason for hiding this comment

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

It's a good improvement.

pkg/bazel/bazel.go Outdated Show resolved Hide resolved
pkg/bazel/bazel.go Show resolved Hide resolved
pkg/bazel/workspace/not_found_error.go Show resolved Hide resolved
@f0rmiga f0rmiga changed the title Refactor bazel refactor: bazel Sep 21, 2022
@cgrindel cgrindel enabled auto-merge (squash) September 21, 2022 22:48
@cgrindel cgrindel merged commit fdbbe62 into main Sep 21, 2022
@cgrindel cgrindel deleted the refactor_bazel branch September 21, 2022 22:51
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