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(output): Ignore sync errors in excluded directories #6460

Conversation

blorente
Copy link
Collaborator

@blorente blorente commented Jun 4, 2024

Checklist

  • I have filed an issue about this change and discussed potential changes with the maintainers.
  • I have received the approval from the maintainers to make this change.
  • This is not a stylistic, refactoring, or cleanup change.

Please note that the maintainers will not be reviewing this change until all checkboxes are ticked. See
the Contributions section in the README for more
details.

Discussion thread for this change

Issue number: N/A

Description of this change

Even if a directory is excluded, Bazel can try to read it and issue
errors during sync. We should ignore Bazel errors that come from
excluded packages.

Implementation details:

  • We create a new issue category, IGNORE.
  • We modify the relevant BlazeIssueParsers to hold references to workspaceRoot and the projectViewSet
  • When parsing an issue, if the issue happens in an excluded package or directory or label, we emit it as IGNORE.

Even if a directory is excluded, Bazel can try to read it and issue
errors during sync. We should ignore Bazel errors that come from
excluded packages.
@github-actions github-actions bot added product: CLion CLion plugin product: IntelliJ IntelliJ plugin product: GoLand GoLand plugin awaiting-review Awaiting review from Bazel team on PRs labels Jun 4, 2024
Copy link
Collaborator

@agluszak agluszak left a comment

Choose a reason for hiding this comment

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

Code-wise LGTM, but feature-wise I don't feel confident enough to approve it

@blorente
Copy link
Collaborator Author

blorente commented Jun 6, 2024

@agluszak fair enough, thanks for taking a look! It is a potentially confusing change to the UX, I appreciate the thoughtfulness.

I'll ping @tpasternak and @mai93 to see if they want the feature, otherwise I'm happy to close the PR.

@blorente
Copy link
Collaborator Author

Synced with @tpasternak online. We'll hold off on landing this because it might hide legitimate errors in third party dependencies.

Our use-case for this is pretty niche (we use it for projects with both Bazel and CMake builds in the same repo), but the right solution is probably to expose another extension point for issue parsers. I'm not going to do that in this PR, so closing this one.

@blorente blorente closed this Jun 10, 2024
@github-actions github-actions bot removed the awaiting-review Awaiting review from Bazel team on PRs label Jun 10, 2024
blorente pushed a commit to blorente/intellij that referenced this pull request Jun 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
product: CLion CLion plugin product: GoLand GoLand plugin product: IntelliJ IntelliJ plugin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants