Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

use absolute directories paths while deriving targets #6544

Merged
merged 4 commits into from
Jul 17, 2024

Conversation

kusaeva
Copy link
Contributor

@kusaeva kusaeva commented Jul 2, 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: #6439

Description of this change

Use absolute paths for directories excluded from project view

Copy link

google-cla bot commented Jul 2, 2024

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@github-actions github-actions bot added the awaiting-review Awaiting review from Bazel team on PRs label Jul 2, 2024
@mai93 mai93 self-assigned this Jul 2, 2024
@mai93
Copy link
Collaborator

mai93 commented Jul 2, 2024

@sgowroji sgowroji added the product: IntelliJ IntelliJ plugin label Jul 3, 2024
@kusaeva
Copy link
Contributor Author

kusaeva commented Jul 3, 2024

Hey, @mai93, thanks for the quick review!

I had to add a new test class with the BlazeQueryDirectoryToTargetProvider.getQueryString check, because that's the only way I managed to fail the test with the old code and pass with the new one.
I have used a temporary virtual file system provider so that I could create some fake file structure to verify the existence of directories included or excluded from the project.

I also had to change test_package_root in intellij_integration_test_suite because I'm testing a protected method from the package com.google.idea.blaze.base.dependencies.

Let me know if it is not ok for you

@kusaeva
Copy link
Contributor Author

kusaeva commented Jul 11, 2024

Hey, @mai93 @tpasternak @agluszak @jastice!
I guess this plugin is not your priority, but still will be glad to get some feedback :)

}

protected static String getQueryString(ImportRoots directories, boolean allowManualTargetsSync) {
protected static String getQueryString(ImportRoots directories, boolean allowManualTargetsSync, WorkspacePathResolver pathResolver) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you make it public and annotate it with @VisibleForTesting instead of changing the package in the test target?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@mai93 mai93 merged commit 4c49b32 into bazelbuild:master Jul 17, 2024
6 checks passed
@github-actions github-actions bot removed the awaiting-review Awaiting review from Bazel team on PRs label Jul 17, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
product: IntelliJ IntelliJ plugin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants