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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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.
39 matches
Mail list logo