Skip to content

[ZEPPELIN-1383][ Interpreters][r-interpreter] remove SparkInterpreter.getSystemDefault and update relative code#1372

Closed
WeichenXu123 wants to merge 2 commits intoapache:masterfrom
WeichenXu123:remove_spark_interpreter_getSystemDefault
Closed

[ZEPPELIN-1383][ Interpreters][r-interpreter] remove SparkInterpreter.getSystemDefault and update relative code#1372
WeichenXu123 wants to merge 2 commits intoapache:masterfrom
WeichenXu123:remove_spark_interpreter_getSystemDefault

Conversation

@WeichenXu123
Copy link
Contributor

What is this PR for?

clean some redundant code:
remove SparkInterpreter.getSystemDefault methods,
and replace it with InterpreterProperty.getValue

What type of PR is it?

Improvement

Todos

N/A

What is the Jira issue?

https://issues.apache.org/jira/browse/ZEPPELIN-1383
remove SparkInterpreter.getSystemDefault and update relative code

How should this be tested?

Existing tests.

Screenshots (if appropriate)

Questions:

  • Does the licenses files need update? No
  • Is there breaking changes for older versions? No
  • Does this needs documentation? No

@WeichenXu123 WeichenXu123 force-pushed the remove_spark_interpreter_getSystemDefault branch from cdea57c to 841b757 Compare August 27, 2016 17:44
@WeichenXu123
Copy link
Contributor Author

cc @jongyoul @Leemoonsoo Thanks!

}

public InterpreterPropertyBuilder add(String name,
String envName,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Vertical align is not recommended. Could you fix it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. Does the code has 100 character per-line limit ? If exceed which format is expected ? thanks~

Copy link
Contributor Author

@WeichenXu123 WeichenXu123 Aug 29, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix it like folllowing:
public InterpreterPropertyBuilder add(String name, String envName, String propertyName, String defaultValue, String description)
thanks!

@jongyoul
Copy link
Member

I think it's enough for my purpose. Thanks. LGTM

@WeichenXu123 WeichenXu123 reopened this Aug 30, 2016
@asfgit asfgit closed this in 223d225 Aug 31, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments