You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by rerngvit <gi...@git.apache.org> on 2015/09/24 00:59:57 UTC

[GitHub] flink pull request: [FLINK-2751] [Documentation] Add quickstart me...

GitHub user rerngvit opened a pull request:

    https://github.com/apache/flink/pull/1176

    [FLINK-2751] [Documentation] Add quickstart menu to the navigation bar

    Add quickstart menu to the navigation bar for Flink documentation

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/rerngvit/flink FLINK-2751

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/flink/pull/1176.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #1176
    
----
commit e588f3067e63fbdf261dedb9913087d1f2fa083c
Author: Rerngvit Yanggratoke <re...@kth.se>
Date:   2015-09-23T22:56:04Z

    Add quickstart menu to the navigation bar

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2751] [Documentation] Add quickstart me...

Posted by rerngvit <gi...@git.apache.org>.
Github user rerngvit commented on the pull request:

    https://github.com/apache/flink/pull/1176#issuecomment-142900768
  
    @chiwanpark Thanks for reviewing this issue. I modified the pull request according to your comments. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2751] [Documentation] Add quickstart me...

Posted by chiwanpark <gi...@git.apache.org>.
Github user chiwanpark commented on the pull request:

    https://github.com/apache/flink/pull/1176#issuecomment-142863133
  
    Hi @rerngvit, Thanks for sending pull request. But your pull request has some problems.
    
    First, `{{ quickstart }}` is not defined in your changes. As you can see in 19-22 lines of `navbar.html`, you should define `{{ quickstart }}`.
    
    Second, there is no conditional `active` class for quickstart dropdown menu. Please include conditional `active` class in quickstart dropdown menu. Conditional `active` classes for other menus are in line 55 of `navbar.html`.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2751] [Documentation] Add quickstart me...

Posted by rerngvit <gi...@git.apache.org>.
Github user rerngvit commented on the pull request:

    https://github.com/apache/flink/pull/1176#issuecomment-142868010
  
    @chiwanpark Thanks for reviewing this issue. I modified the pull request according to your comments. Please have a look.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2751] [Documentation] Add quickstart me...

Posted by mxm <gi...@git.apache.org>.
Github user mxm commented on the pull request:

    https://github.com/apache/flink/pull/1176#issuecomment-143187594
  
    Looks good. Thanks for the contribution @rerngvit!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2751] [Documentation] Add quickstart me...

Posted by chiwanpark <gi...@git.apache.org>.
Github user chiwanpark commented on the pull request:

    https://github.com/apache/flink/pull/1176#issuecomment-142910324
  
    Looks good to merge. I'll merge this.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2751] [Documentation] Add quickstart me...

Posted by chiwanpark <gi...@git.apache.org>.
Github user chiwanpark commented on the pull request:

    https://github.com/apache/flink/pull/1176#issuecomment-142895387
  
    Hi @rerngvit, I have tested your pull request.
    
    Because `setup_quickstart.html` contains "setup" keyword in url, there are two active menus (Quickstart, Setup) when we open Quickstart: Setup page. I found a solution like following:
    
    ```html
    <li class="dropdown{% if page.url contains '/quickstart/' %} active{% endif %}">
    ```
    
    The solution must be applied not only quickstart but also other dropdown menus such as setup, programming guide, ..., etc..
    
    BTW, there is a typo line 68 of `navbar.html`. A tag must be closed. I know this is not related your changes but fixing it would be better.
    
    After addressing this, we can merge this pull request. :-)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-2751] [Documentation] Add quickstart me...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/flink/pull/1176


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---