-
Notifications
You must be signed in to change notification settings - Fork 302
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
feat: create "Add package" EditorNotificationPanel if derive_targets_from_directories is on #6388
Conversation
…er based on the presence of `BUILD` file there
…done now when we add a file to project.
Also, split the code in earlier phase
So initially there was a notification
And with this change it in some cases looks like
I'd consider updating notification text for the new action into something like
Also I checked the code on a basic example with Screencast.from.2024-04-18.16-33-45.webm |
edit.apply(); | ||
// TODO(brendandouglas): support partially syncing a directory with the same query-based | ||
// filtering | ||
List<? extends TargetExpression> targetsToSync = targets; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is always null
return null; | ||
} | ||
EditorNotificationPanel panel = | ||
createPanel( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems to me that some regions have formatting issues. I believe the indent should be 4 spaces instead of 8.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, but agreed to extract duplicated constant to variable in a separate PR
Based on #6359 by @dkashyn-sfdc
Checklist
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