Skip to content
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

[DOC] Improve documents related to Helium #2236

Closed
wants to merge 2 commits into from

Conversation

tae-jun
Copy link
Contributor

@tae-jun tae-jun commented Apr 9, 2017

What is this PR for?

What I did for the documents:

  • Highlight codes
  • Follow JSON syntax
  • Remove white spaces

And in my opinion, here is ambiguous:

"Normally, you can add any dependencies in package.json however Zeppelin Visualization package only allows two dependencies: zeppelin-vis and zeppelin-tabledata."

Does it want to say "you can add any dependencies in package.json, but you must include two dependencies: zeppelin-vis and zeppelin-tabledata."?

What type of PR is it?

[Documentation]

Questions:

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

@AhyoungRyu
Copy link
Contributor

@tae-jun Thanks for the improvement.

You can add any dependencies in package.json, but you must include two dependencies: zeppelin-vis and zeppelin-tabledata.

Yeah I think you're right.

@tae-jun
Copy link
Contributor Author

tae-jun commented Apr 10, 2017

@AhyoungRyu Thanks for the review!

I changed the sentence, and now it looks like this:

You can add any dependencies in package.json, but you must include two dependencies: zeppelin-vis and zeppelin-tabledata.

I marked it as bold, since I think it's important 😄

@AhyoungRyu
Copy link
Contributor

@tae-jun Looks GOOD to merge :)

@tae-jun
Copy link
Contributor Author

tae-jun commented Apr 10, 2017

@AhyoungRyu Thanks! 👍

@AhyoungRyu
Copy link
Contributor

Merge into master if there are no more comments on this.

@asfgit asfgit closed this in be20201 Apr 17, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants