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 extension point to detect whether a module is to be orphaned or retained. #6467

Merged
merged 1 commit into from
Jun 13, 2024

Conversation

gchandu25
Copy link
Contributor

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: <please reference the issue number or url here>

Description of this change

Added an EP to give the feasibility to allow other modules. For example, when maven modules are added into project view bazel sync resets the view as they have different content root. An implementation could be added to allow maven modules to be shown in project tree view.

@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 5, 2024
@gchandu25 gchandu25 force-pushed the bazel-sync-keep-mavenModules branch from c220215 to c9d51b0 Compare June 9, 2024 07:39
ExtensionPointName<ExternalModuleProvider> EP_NAME =
ExtensionPointName.create("com.google.idea.blaze.base.sync.projectstructure.ExternalModuleProvider");

boolean isOwnedByExternalModule(Module module);
Copy link
Contributor

Choose a reason for hiding this comment

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

it is not really externalModule that own this module but it is much better than validation so I'll approve (not that it matters :) )

Copy link
Collaborator

Choose a reason for hiding this comment

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

yeah, if it was an internal name I would change it afterwards, but this is an EP, so let's avoid changes once its merged. Could you please change that to isOwnedByExternalPlugin or isOwnedByThirdPartyPlugin?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated to 'isOwnedByExternalPlugin'. Please a take @tpasternak.

Copy link
Contributor

@dkashyn-sfdc dkashyn-sfdc left a comment

Choose a reason for hiding this comment

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

LGTM

ExtensionPointName<ExternalModuleProvider> EP_NAME =
ExtensionPointName.create("com.google.idea.blaze.base.sync.projectstructure.ExternalModuleProvider");

boolean isOwnedByExternalModule(Module module);
Copy link
Collaborator

Choose a reason for hiding this comment

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

yeah, if it was an internal name I would change it afterwards, but this is an EP, so let's avoid changes once its merged. Could you please change that to isOwnedByExternalPlugin or isOwnedByThirdPartyPlugin?

@gchandu25 gchandu25 force-pushed the bazel-sync-keep-mavenModules branch from e9f7069 to b899b34 Compare June 12, 2024 17:30
@jastice jastice dismissed tpasternak’s stale review June 13, 2024 12:58

changes addressed

@jastice jastice merged commit 56fc692 into bazelbuild:master Jun 13, 2024
6 checks passed
@github-actions github-actions bot removed the awaiting-review Awaiting review from Bazel team on PRs label 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.

None yet

6 participants