-
Notifications
You must be signed in to change notification settings - Fork 305
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
Use git info only when Expand to Working Set is enabled #6944
Use git info only when Expand to Working Set is enabled #6944
Conversation
vcsState.flatMap(s -> s.workspaceSnapshotPath).map(Object::toString).orElse("<none>"))); | ||
|
||
Optional<VcsState> vcsState = Optional.empty(); | ||
if (BlazeUserSettings.getInstance().getExpandSyncToWorkingSet()) { |
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.
Probably other option should be added for QS yet even QS-relevant delta calculation will be taxing in some cases.
Also Sync to Working Set is enabled by default, iirc. So default QS behavior won't change.
@@ -125,7 +126,7 @@ private SyncProjectState getProjectState(BlazeContext context, BlazeSyncParams p | |||
createBazelInfoFuture(context, syncFlags, params.syncMode()); | |||
|
|||
ListenableFuture<WorkingSet> workingSetFuture; | |||
if (params.addWorkingSet() || params.syncMode() == SyncMode.FULL) { | |||
if (params.addWorkingSet() || (BlazeUserSettings.getInstance().getExpandSyncToWorkingSet() && params.syncMode() == SyncMode.FULL)) { |
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.
Don't know when this one is used but it is bad to ignore the setting anyway so we need to double check here.
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.
Oh, params.addWorkingSet already checks for that, but I'll merge
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.
I want to avoid unconditional params.syncMode() == SyncMode.FULL
this is quite recent change from September
56b104c
to
087553f
Compare
@@ -125,7 +126,7 @@ private SyncProjectState getProjectState(BlazeContext context, BlazeSyncParams p | |||
createBazelInfoFuture(context, syncFlags, params.syncMode()); | |||
|
|||
ListenableFuture<WorkingSet> workingSetFuture; | |||
if (params.addWorkingSet() || params.syncMode() == SyncMode.FULL) { | |||
if (params.addWorkingSet() || (BlazeUserSettings.getInstance().getExpandSyncToWorkingSet() && params.syncMode() == SyncMode.FULL)) { |
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.
Oh, params.addWorkingSet already checks for that, but I'll merge
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: (#6943)
Description of this change