Skip to content

Fix dead link and add table tab to Zeppelin Configuration section: docs/install/install.md#679

Closed
AhyoungRyu wants to merge 8 commits into
apache:masterfrom
AhyoungRyu:ZEPPELIN-638
Closed

Fix dead link and add table tab to Zeppelin Configuration section: docs/install/install.md#679
AhyoungRyu wants to merge 8 commits into
apache:masterfrom
AhyoungRyu:ZEPPELIN-638

Conversation

@AhyoungRyu
Copy link
Copy Markdown
Contributor

What is this PR for?

As mentioned at ZEPPELIN-638, there is a dead link in docs/install/install.md. So I fixed it. Plus, I also add a table tab to Zeppelin Configuration section because current one is not good to look.

What type of PR is it?

Improvement | Documentation

Todos

  • - Fix dead link
  • - Add table tab to Zeppelin Configuration section

Is there a relevant Jira issue?

ZEPPELIN-638 (only for fixing dead link)

How should this be tested?

After applying this PR, you can check this page Quick Start -> Install at Zeppelin documentation.

Screenshots (if appropriate)

  • Before
    before
  • After
    after

Questions:

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

@felixcheung
Copy link
Copy Markdown
Member

The tabs are nice but would it be better to put -env.sh in a column, -site.xml in another in one table? They have a lot overlap?

@AhyoungRyu
Copy link
Copy Markdown
Contributor Author

@felixcheung You're right.
But I've thought this table is too big to identify at a glance :’-(
Then, how about adding a note sentence below( or above ) the table such as "There are a lot of overlapped facts between zeppelin-env.sh and zeppelin-site.xml. Please compare and use what you need."
It's just my personal opinion, if two columns in one table are better, I'm willing to set back : )

@Leemoonsoo
Copy link
Copy Markdown
Member

Thanks @AhyoungRyu for the improvement. But I also think seeing environment variable and property in one table together is much easier to discover necessary configuration.

@AhyoungRyu
Copy link
Copy Markdown
Contributor Author

@Leemoonsoo I see. So I applied your comment and pushed it : )

Comment thread docs/install/install.md Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

extra space in "from Zeppelin"

@AhyoungRyu
Copy link
Copy Markdown
Contributor Author

@felixcheung
I really appreciate to your detailed explanation! I applied your comments and pushed again : )

Comment thread docs/install/install.md Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

In this documentation, we will guide you how you can install
-> In this documentation, we will explain how you can install

Plus, you can get a specific infomation about Zeppelin configurations at the below Zeppelin Configuration section
-> Plus, you can see all of Zeppelin's configurations in the Zeppelin Configuration section below.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thank you for the review @corneadoug : )
I applied your comments and pushed again.

@corneadoug
Copy link
Copy Markdown
Contributor

Except for that last comment, LGTM!

@AhyoungRyu
Copy link
Copy Markdown
Contributor Author

@corneadoug Thanks. I applied it : )

@corneadoug
Copy link
Copy Markdown
Contributor

Merging if there is no more discussions

@AhyoungRyu AhyoungRyu mentioned this pull request Feb 4, 2016
@asfgit asfgit closed this in 025acd3 Feb 4, 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.

4 participants