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

Allow process_executor to make a JDK present #6443

Merged
merged 2 commits into from Sep 5, 2018

Conversation

Projects
None yet
4 participants
@illicitonion
Copy link
Contributor

illicitonion commented Sep 3, 2018

No description provided.

@illicitonion illicitonion requested review from blorente and dotordogh Sep 3, 2018

@blorente
Copy link
Contributor

blorente left a comment

Looks good! Thanks

.long("jdk")
.takes_value(true)
.required(false)
.help("Symlink a JDK from .jdk in the working directory. For local execution, symlinks to the value of this flag. For remote execution, just requests that some JDK is symlinked if this flag has any value.")

This comment has been minimized.

@blorente

blorente Sep 3, 2018

Contributor

I'm not sure I like the fact that we only care about the value of the flag when we run locally, and ignore it when we remote. But, I can't think of a better way to do it, so it will probably have to be this way.

This comment has been minimized.

@stuhood

stuhood Sep 4, 2018

Member

Worth linking to the github issues we have about replacing this situation in as many places as possible.

@stuhood

stuhood approved these changes Sep 4, 2018

Copy link
Member

stuhood left a comment

Thanks.

.long("jdk")
.takes_value(true)
.required(false)
.help("Symlink a JDK from .jdk in the working directory. For local execution, symlinks to the value of this flag. For remote execution, just requests that some JDK is symlinked if this flag has any value.")

This comment has been minimized.

@stuhood

stuhood Sep 4, 2018

Member

Worth linking to the github issues we have about replacing this situation in as many places as possible.

@illicitonion illicitonion merged commit f631e1b into pantsbuild:master Sep 5, 2018

1 check was pending

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details

@illicitonion illicitonion deleted the twitter:dwagnerhall/process_executor/jdk branch Sep 5, 2018

@baroquebobcat

This comment has been minimized.

Copy link
Contributor

baroquebobcat commented Sep 6, 2018

Cool!

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