[GitHub] zeppelin issue #2233: [MINOR] Add paragraph to note
Github user AlexanderShoshin commented on the issue: https://github.com/apache/zeppelin/pull/2233 Hi Felix, thank you for looking through my PR. Method name must correspond an action it performs. Methods with names like `addSomething()` usually mean that we must pass this _something_ into the method as an input parameter. I have renamed an `addParagraph()` method because it doesn't receive a `Paragraph` instance but creates a new one instead. This also gave me an opportunity to add a new `addParagraph()` method which does receive a `Paragraph` instance. As for the `Note.addNewParagraph()` method name it seem to me that it sounds like "create new paragraph and add it to the `Note`". But if such name is still not clear maybe we should choose another one like `createNewParagraph()` or `createAndAddParagraph()` or something like this. What do you think? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #2233: [MINOR] Add paragraph to note
Github user felixcheung commented on the issue: https://github.com/apache/zeppelin/pull/2233 sounds reasonable. are your TO-DO completed and this PR ready for review? If not, it's customary to have `[WIP]` in your PR title. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #2233: [MINOR] Add paragraph to note
Github user AlexanderShoshin commented on the issue: https://github.com/apache/zeppelin/pull/2233 This PR is ready for review. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #2233: [MINOR] Add paragraph to note
Github user jongyoul commented on the issue: https://github.com/apache/zeppelin/pull/2233 LGTM --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #2233: [MINOR] Add paragraph to note
Github user Leemoonsoo commented on the issue: https://github.com/apache/zeppelin/pull/2233 LGTM and merge to master if no further discussions. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---