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 support for passing an incremental_import option via idea-plugin #5886

Merged
merged 2 commits into from Jun 4, 2018

Conversation

Projects
None yet
2 participants
@stuhood
Copy link
Member

stuhood commented May 30, 2018

Problem

The "incremental import" feature avoids loading all of the transitive deps of a project into IntelliJ, but it is not currently supported by the ./pants idea-plugin launcher.

Solution

Add support for the option (defaulting to disabled, to minimize confusion).

Result

A change in the http://github.com/pantsbuild/intellij-pants-plugin/ repo will be able to consume it.

@stuhood stuhood requested review from wisechengyi and cosmicexplorer May 30, 2018

@@ -27,7 +27,7 @@
_TEMPLATE_BASEDIR = 'templates/idea'

# Follow `export.py` for versioning strategy.
IDEA_PLUGIN_VERSION = '0.0.2'
IDEA_PLUGIN_VERSION = '1.9.2'

This comment has been minimized.

@wisechengyi

wisechengyi May 31, 2018

Contributor

Any reason for 1.9.2? I was thinking 0.0.3 because

# Patch version x.x.1 : Increment this field when a minor format change that just adds information
# that an application can safely ignore.

This comment has been minimized.

@stuhood

stuhood May 31, 2018

Member

Hm, despite the comment, I was still thinking that this meant the intellij pants plugin. The next release of that is 1.9.2, ergo...

Will fix.

@@ -63,6 +63,8 @@ def register_options(cls, register):
# scala/java-language level should use what Pants already knows.
register('--open', type=bool, default=True,
help='Attempts to open the generated project in IDEA.')
register('--incremental-import', type=int, default=None,
help='Enable incremental import of targets with the given graph depth.')

This comment has been minimized.

@wisechengyi

wisechengyi May 31, 2018

Contributor

I wasn't sure about specifying depth number directly, as opposed to a boolean, because currently there's no way to specify depth prior to pants export https://github.com/pantsbuild/intellij-pants-plugin/blob/43ec02d80c1a87291c6533ea35997cd82afbba88/src/com/twitter/intellij/pants/components/impl/PantsProjectComponentImpl.java#L136

whereas depth number is now collected via GUI after pants export https://github.com/pantsbuild/intellij-pants-plugin/blob/3027a708a87f993590354383d90a1f6872e92dd3/src/com/twitter/intellij/pants/service/project/resolver/PantsCreateModulesExtension.java#L93-L115 (yes a little awkward there)

That said, it's good to set this up first, then get the awkwardness fixed afterwards, but do you think it would be a good idea to clarify in the help msg?

Enable incremental import of targets with the given graph depth. 
Depending on the version of IntelliJ Pants plugin, depth info may be prompted again or ignored.

This comment has been minimized.

@stuhood

stuhood Jun 1, 2018

Member

Clarified the message.

@wisechengyi
Copy link
Contributor

wisechengyi left a comment

Thanks @stuhood !

@wisechengyi wisechengyi merged commit c33ccd4 into pantsbuild:master Jun 4, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@wisechengyi wisechengyi deleted the twitter:stuhood/idea-plugin-supports-incremental-import branch Jun 4, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment