[GitHub] zeppelin issue #1730: [ZEPPELIN-1736] Introduce trash & enable removing fold...

2017-01-01 Thread tae-jun
Github user tae-jun commented on the issue: https://github.com/apache/zeppelin/pull/1730 @AhyoungRyu Finally, it's green!!! Thanks for your care 😄 --- 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 n

[GitHub] zeppelin issue #1730: [ZEPPELIN-1736] Introduce trash & enable removing fold...

2017-01-01 Thread AhyoungRyu
Github user AhyoungRyu commented on the issue: https://github.com/apache/zeppelin/pull/1730 1 build failed with below error, but it's irrelevant to this change. ``` [ERROR] Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:2.17:test (default-test) on

[GitHub] zeppelin issue #1730: [ZEPPELIN-1736] Introduce trash & enable removing fold...

2016-12-31 Thread tae-jun
Github user tae-jun commented on the issue: https://github.com/apache/zeppelin/pull/1730 @AhyoungRyu Thanks!!! I fixed the test error thanks to you 👍 I guess CI will be green this time 😄 --- If your project is set up for it, you can reply to this email and have your r

[GitHub] zeppelin issue #1730: [ZEPPELIN-1736] Introduce trash & enable removing fold...

2016-12-31 Thread AhyoungRyu
Github user AhyoungRyu commented on the issue: https://github.com/apache/zeppelin/pull/1730 @tae-jun I tried to run `npm run test` under `zeppelin-web` in my local, but it failed with below error. ``` PhantomJS 2.1.1 (Mac OS X 0.0.0) Factory: NoteList should generate both fla

[GitHub] zeppelin issue #1730: [ZEPPELIN-1736] Introduce trash & enable removing fold...

2016-12-31 Thread AhyoungRyu
Github user AhyoungRyu commented on the issue: https://github.com/apache/zeppelin/pull/1730 @tae-jun Can you try to rebase from master and push again? I guess it would be help to make CI all green. Seems it's because of #1805. --- If your project is set up for it, you can reply to t

[GitHub] zeppelin issue #1730: [ZEPPELIN-1736] Introduce trash & enable removing fold...

2016-12-31 Thread tae-jun
Github user tae-jun commented on the issue: https://github.com/apache/zeppelin/pull/1730 Thanks @AhyoungRyu 😄 --- 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 wish

[GitHub] zeppelin issue #1730: [ZEPPELIN-1736] Introduce trash & enable removing fold...

2016-12-31 Thread AhyoungRyu
Github user AhyoungRyu commented on the issue: https://github.com/apache/zeppelin/pull/1730 I think it's good to merge. So let's improve after then! Will merge this one if there are no further discussions. @tae-jun Appreciate for this AWESOME feature 👍 --- If your project i

[GitHub] zeppelin issue #1730: [ZEPPELIN-1736] Introduce trash & enable removing fold...

2016-12-30 Thread soralee
Github user soralee commented on the issue: https://github.com/apache/zeppelin/pull/1730 Thanks for considering my suggestion, @tae-jun 😄 It is good idea and I agree with you! I think this problem is a little too hard to solve. So, anyway, I will test again! --- If yo

[GitHub] zeppelin issue #1730: [ZEPPELIN-1736] Introduce trash & enable removing fold...

2016-12-30 Thread tae-jun
Github user tae-jun commented on the issue: https://github.com/apache/zeppelin/pull/1730 Thanks for the review, @soralee! That was a big deal. I pushed a commit and now it looks like this: https://cloud.githubusercontent.com/assets/8201019/21564175/bc5fc4c2-cecd-11e6-88dd-d

[GitHub] zeppelin issue #1730: [ZEPPELIN-1736] Introduce trash & enable removing fold...

2016-12-27 Thread soralee
Github user soralee commented on the issue: https://github.com/apache/zeppelin/pull/1730 @tae-jun It's great feature! So, I would like to suggest one thing. Currently, the filter function in Zeppelin homepage doesn't work as expected and this issue is handled in [ZEPPELIN-1864](ht

[GitHub] zeppelin issue #1730: [ZEPPELIN-1736] Introduce trash & enable removing fold...

2016-12-27 Thread AhyoungRyu
Github user AhyoungRyu commented on the issue: https://github.com/apache/zeppelin/pull/1730 @tae-jun Thanks for your great work! Merge if there are no more discussions :) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. I

[GitHub] zeppelin issue #1730: [ZEPPELIN-1736] Introduce trash & enable removing fold...

2016-12-27 Thread Leemoonsoo
Github user Leemoonsoo commented on the issue: https://github.com/apache/zeppelin/pull/1730 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

[GitHub] zeppelin issue #1730: [ZEPPELIN-1736] Introduce trash & enable removing fold...

2016-12-25 Thread tae-jun
Github user tae-jun commented on the issue: https://github.com/apache/zeppelin/pull/1730 Conflict resolved 😄 --- 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

[GitHub] zeppelin issue #1730: [ZEPPELIN-1736] Introduce trash & enable removing fold...

2016-12-23 Thread tae-jun
Github user tae-jun commented on the issue: https://github.com/apache/zeppelin/pull/1730 @AhyoungRyu Yeah! Thanks :) Merry Christmas~ --- 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 feat

[GitHub] zeppelin issue #1730: [ZEPPELIN-1736] Introduce trash & enable removing fold...

2016-12-23 Thread AhyoungRyu
Github user AhyoungRyu commented on the issue: https://github.com/apache/zeppelin/pull/1730 Finally CI is green now! 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

[GitHub] zeppelin issue #1730: [ZEPPELIN-1736] Introduce trash & enable removing fold...

2016-12-23 Thread tae-jun
Github user tae-jun commented on the issue: https://github.com/apache/zeppelin/pull/1730 Thanks @1ambda 😄 --- 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

[GitHub] zeppelin issue #1730: [ZEPPELIN-1736] Introduce trash & enable removing fold...

2016-12-23 Thread 1ambda
Github user 1ambda commented on the issue: https://github.com/apache/zeppelin/pull/1730 @tae-jun great! :) --- 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,

[GitHub] zeppelin issue #1730: [ZEPPELIN-1736] Introduce trash & enable removing fold...

2016-12-22 Thread tae-jun
Github user tae-jun commented on the issue: https://github.com/apache/zeppelin/pull/1730 @1ambda Thanks for your prompt response! I see what you mean now. Thanks for the explanation :) Now, only `remove permanently` and `restore` buttons are shown. https://cloud.githubu

[GitHub] zeppelin issue #1730: [ZEPPELIN-1736] Introduce trash & enable removing fold...

2016-12-22 Thread 1ambda
Github user 1ambda commented on the issue: https://github.com/apache/zeppelin/pull/1730 Hi @tae-jun It's good idea to introduce limitation when notes are in trash. Let me comment (1) what you mentioned is about restricting modification of archived notes (i mean, not

[GitHub] zeppelin issue #1730: [ZEPPELIN-1736] Introduce trash & enable removing fold...

2016-12-22 Thread tae-jun
Github user tae-jun commented on the issue: https://github.com/apache/zeppelin/pull/1730 @AhyoungRyu Always thanks for the review :) Also thanks for your understanding. @1ambda Thanks for the review! I appreciate it and sorry for my late response. That was a really nice

[GitHub] zeppelin issue #1730: [ZEPPELIN-1736] Introduce trash & enable removing fold...

2016-12-19 Thread AhyoungRyu
Github user AhyoungRyu commented on the issue: https://github.com/apache/zeppelin/pull/1730 @tae-jun Thanks for your precise explanation. Yeah it makes sense. And tested again and it looks good for what we talked about :) It'll be nice to be merged after addressing @1ambda's

[GitHub] zeppelin issue #1730: [ZEPPELIN-1736] Introduce trash & enable removing fold...

2016-12-18 Thread 1ambda
Github user 1ambda commented on the issue: https://github.com/apache/zeppelin/pull/1730 @tae-jun It works well as described and here are two things to consider. 1. Trash is represented in `~Trash` in the Navbar. https://cloud.githubusercontent.com/assets/496

[GitHub] zeppelin issue #1730: [ZEPPELIN-1736] Introduce trash & enable removing fold...

2016-12-18 Thread 1ambda
Github user 1ambda commented on the issue: https://github.com/apache/zeppelin/pull/1730 Hi all, This PR looks like unpopular from the point of view design. I will close this issue. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub

[GitHub] zeppelin issue #1730: [ZEPPELIN-1736] Introduce trash & enable removing fold...

2016-12-17 Thread tae-jun
Github user tae-jun commented on the issue: https://github.com/apache/zeppelin/pull/1730 I changed `empty trash`, `remove folder permanently`, and `remove note permanently` icons to `X`. I'm not sure this is the best icon for it ^^; but we can change later when we find a suitable icon

[GitHub] zeppelin issue #1730: [ZEPPELIN-1736] Introduce trash & enable removing fold...

2016-12-14 Thread AhyoungRyu
Github user AhyoungRyu commented on the issue: https://github.com/apache/zeppelin/pull/1730 @tae-jun Appreciate for your precise explanation and picking proper candidates as well :) Tested again to check the time stamp, it makes sense. Then how about attaching the time stamp for ever

[GitHub] zeppelin issue #1730: [ZEPPELIN-1736] Introduce trash & enable removing fold...

2016-12-14 Thread tae-jun
Github user tae-jun commented on the issue: https://github.com/apache/zeppelin/pull/1730 @AhyoungRyu Always thanks for your nice review 😄 Yes, it's intended. A time stamp is generated only if a `folder` already exists in the trash. But it's not generated when moving a `not

[GitHub] zeppelin issue #1730: [ZEPPELIN-1736] Introduce trash & enable removing fold...

2016-12-13 Thread AhyoungRyu
Github user AhyoungRyu commented on the issue: https://github.com/apache/zeppelin/pull/1730 @tae-jun Nice feature indeed! Most of cases are working like a charm. But as @marchpig said above, ![screen shot 2016-12-14 at 12 28 46 pm](https://cloud.githubusercontent.com/asse

[GitHub] zeppelin issue #1730: [ZEPPELIN-1736] Introduce trash & enable removing fold...

2016-12-13 Thread tae-jun
Github user tae-jun commented on the issue: https://github.com/apache/zeppelin/pull/1730 Thanks @Leemoonsoo 😄 --- 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 wish

[GitHub] zeppelin issue #1730: [ZEPPELIN-1736] Introduce trash & enable removing fold...

2016-12-13 Thread Leemoonsoo
Github user Leemoonsoo commented on the issue: https://github.com/apache/zeppelin/pull/1730 @tae-jun thanks for the great contribution! LGTM and merge to master if there're no further discussions. --- If your project is set up for it, you can reply to this email and have your repl

[GitHub] zeppelin issue #1730: [ZEPPELIN-1736] Introduce trash & enable removing fold...

2016-12-11 Thread tae-jun
Github user tae-jun commented on the issue: https://github.com/apache/zeppelin/pull/1730 ### CI is green! 😄 I **changed Selenium test codes** affected by my PR and it works now. Failed reason `remove note` button changed to `move to trash` but test code still w

[GitHub] zeppelin issue #1730: [ZEPPELIN-1736] Introduce trash & enable removing fold...

2016-12-11 Thread tae-jun
Github user tae-jun commented on the issue: https://github.com/apache/zeppelin/pull/1730 Thanks for your careful review @marchpig 😄 I fixed the bug you found! It turns out that I didn't normalize folder ID when creating a folder. It could be a serious bug if you didn't find

[GitHub] zeppelin issue #1730: [ZEPPELIN-1736] Introduce trash & enable removing fold...

2016-12-09 Thread marchpig
Github user marchpig commented on the issue: https://github.com/apache/zeppelin/pull/1730 @tae-jun It works well! Thank you very much for accepting my opinions. :smiley: I think that handling `~Trash` prefix in the future is fine to me because it is not a bug but just my opinion

[GitHub] zeppelin issue #1730: [ZEPPELIN-1736] Introduce trash & enable removing fold...

2016-12-08 Thread tae-jun
Github user tae-jun commented on the issue: https://github.com/apache/zeppelin/pull/1730 @marchpig Sorry for my late response! I changed what you addressed except one thing. ### Korean note order ![image](https://cloud.githubusercontent.com/assets/8201019/21017141/b8b797e

[GitHub] zeppelin issue #1730: [ZEPPELIN-1736] Introduce trash & enable removing fold...

2016-12-07 Thread tae-jun
Github user tae-jun commented on the issue: https://github.com/apache/zeppelin/pull/1730 Thanks @cuspymd 😄 --- 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

[GitHub] zeppelin issue #1730: [ZEPPELIN-1736] Introduce trash & enable removing fold...

2016-12-06 Thread cuspymd
Github user cuspymd commented on the issue: https://github.com/apache/zeppelin/pull/1730 @tae-jun Yes. Good Idea~!! --- 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 wi

[GitHub] zeppelin issue #1730: [ZEPPELIN-1736] Introduce trash & enable removing fold...

2016-12-06 Thread tae-jun
Github user tae-jun commented on the issue: https://github.com/apache/zeppelin/pull/1730 Thanks for the review @cuspymd! I think so, but I used HashSet because of consistency. Other methods used HashSet. I think if we want to change to 'Set', wouldn't it be better to open new

[GitHub] zeppelin issue #1730: [ZEPPELIN-1736] Introduce trash & enable removing fold...

2016-12-06 Thread tae-jun
Github user tae-jun commented on the issue: https://github.com/apache/zeppelin/pull/1730 ![image](https://cloud.githubusercontent.com/assets/8201019/20929471/8897948a-bc0d-11e6-941b-4123bc09c8a7.png) It failed only at Selenium test twice. I guess it's because I **changed the actio

[GitHub] zeppelin issue #1730: [ZEPPELIN-1736] Introduce trash & enable removing fold...

2016-12-06 Thread tae-jun
Github user tae-jun commented on the issue: https://github.com/apache/zeppelin/pull/1730 Thanks, @1ambda 😄 --- 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 s

[GitHub] zeppelin issue #1730: [ZEPPELIN-1736] Introduce trash & enable removing fold...

2016-12-06 Thread 1ambda
Github user 1ambda commented on the issue: https://github.com/apache/zeppelin/pull/1730 Let me review and give some feedbacks :) Thanks for implementing the useful feature! --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well.