Skip to content

[ZEPPELIN-49] Interpreter.getProperty returns wrong value - #41

Closed
jongyoul wants to merge 1 commit into
apache:masterfrom
jongyoul:ZEPPELIN-49
Closed

[ZEPPELIN-49] Interpreter.getProperty returns wrong value#41
jongyoul wants to merge 1 commit into
apache:masterfrom
jongyoul:ZEPPELIN-49

Conversation

@jongyoul

Copy link
Copy Markdown
Member
  • Fixed to return p, not property

@RamVenkatesh

Copy link
Copy Markdown
Contributor

LGTM!

@jongyoul

Copy link
Copy Markdown
MemberAuthor

@RamVenkatesh Thanks.

@bzz

bzz commented Apr 17, 2015

Copy link
Copy Markdown
Member

Looks great!
Any way to test it to avoid further errors, by the way?

@jongyoul

Copy link
Copy Markdown
MemberAuthor

@bzz Do you want to test the method of Interpreter.getProperty itself? It can help make sample subclass of Interpreter including static register codes and then make an instance with some properties and and them manually. Do you think it is good?

@bzz

bzz commented Apr 17, 2015

Copy link
Copy Markdown
Member

@jongyoul That would be great, but this sounds too complicated for such a change.. Do you think it worth doing now?

@jongyoul

Copy link
Copy Markdown
MemberAuthor

If we have a plan to change an implementation of Interpreter, it would be good. Otherwise, it's a little tricky. Please tell me your plan.

@bzz

bzz commented Apr 17, 2015

Copy link
Copy Markdown
Member

I agree, lets address it in separate PR later, in case we actually need it.

@bzz

bzz commented Apr 17, 2015

Copy link
Copy Markdown
Member

Merging this one if there is no discussion

Sign up for freeto 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.

3 participants

@jongyoul@RamVenkatesh@bzz