-
Notifications
You must be signed in to change notification settings - Fork 304
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
Added build folder action #6615
Added build folder action #6615
Conversation
You can't build folder, you can only sync it. Building could be much faster. For example in our project sync of entire folder takes x2 time more than build. I've already added this feature to my local Bazel plugin. I've added button to compile directory below the partial sync. It would be great if we can contribute it
…/kitbuilder587/intellij into zajdelovm-feature-request-branch
@@ -0,0 +1,73 @@ | |||
/* | |||
* Copyright 2016 The Bazel Authors. All rights reserved. |
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.
Copyright 2024
import java.util.Map; | ||
import java.util.Objects; | ||
|
||
import java.util.*; |
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.
We prefer to aviod wildcard imports
@@ -111,6 +109,9 @@ private enum ShardingApproach { | |||
|
|||
private static ShardingApproach getShardingApproach( | |||
SyncStrategy parallelStrategy, ProjectViewSet viewSet) { | |||
if (parallelStrategy == SyncStrategy.SERIAL_NOT_EXPAND) { |
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.
So this overrides the shard_sync
setting in the PV, how about just keeping the old behavior 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.
The issue here is that compile folder with shard_sync=true
takes sooo much more time. For example, one of our project's folder compiles in 17s with shard_sync=true
and in 1s with shard_sync=false
. The main reason why we added this button was that sync time was very slow, and we wanted to speed up compile errors check. Ordinary user won't set shard_sync
to false without any reason. We can add another setting to set shard_sync
for build folder action, but I think in cases when OOM happens, you can just use Partial Sync instead
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, that's a good point. Btw what do you think about setting the default PV in tools/intellij/.managed.bazelproject
with shard_sync: true
?.
Example here https://github.com/bazelbuild/bazel/blob/master/tools/intellij/.managed.bazelproject
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.
Personally, I think It's a good idea, but I don't have a lot of knowledge about how often people want to use shard_sync=true
. In our project we use it quite rarely, but I can't expand my PV on all users of the plugin.
Should I create another PR here or we can stay on current realization or may be another ideas?
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.
@LeFrosch what do you think about it?
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, actually if I am not mistaken, shard-sync=false
by default. We can delete this property in enum
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.
But anyway logically it would be better if shard-sync won't affect build. Oftenshard-sync=true
is used for full sync of project if OOM happens. shard-sync=true
will affect the build folder speed. (17s and 1s is big diff) I don't think that it will be used for the whole project often. With shard-sync=true
build folder action is useless, because the diff between build and sync time isn't big. I think SERIAL_NOT_EXPAND anyway will be better
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.
Okay, but I can't guarantee it will be permanent. If it turns out to be misleading, we’ll adjust it to align with the shard_sync setting.
ActionPresentationHelper.of(e) | ||
.disableIf(virtualFile == null) | ||
.disableIf(!virtualFile.isDirectory()) | ||
.setText(virtualFile.getName() + "/...:all") |
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.
No the text is "folder/...alll", can we change it to "Compile folder/...all"?
Project Status API integration with bazel for CLion (bazelbuild#6585) Integration with the new Project Status Widget that is added in CLion 242. Implement py imports handling (bazelbuild#6606) Some `py_...` rules allow for an `imports` attribute. The attribute allows control over how the Python files are vended in terms of modules. This change will mean that the use of the `imports` attribute in Bazel targets is reflected in how the modules are auto-completed and recognized in the IDE. Update CompileCorrespondingBuildFilesAction.java button name change Update CompileCorrespondingBuildFilesAction.java Added build folder action Update BlazeBuildService.java review remarks fix
…/kitbuilder587/intellij into zajdelovm-feature-request-branch
…/kitbuilder587/intellij into zajdelovm-feature-request-branch
f6f9515
to
9f87422
Compare
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:
#6613
Description of this change
Added action to compile folder