-
Notifications
You must be signed in to change notification settings - Fork 2.8k
[ZEPPELIN-1162] Fix rawType in NotebookRestApi #1341
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Please review this PR and let me your thoughts in comments |
|
I think it's enough. LGTM |
| public Response putNotePermissions(@PathParam("noteId") String noteId, String req) | ||
| throws IOException { | ||
| /** | ||
| * TODO(jl): Fixed the type of HashSet |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rajarajan-g Should we also need to remove comments here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you handle it? This PR already is closed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, I have opened another PR for this.
### What is this PR for? Since ZEPPELIN-1162 (PR #1341) has been solved, we should remove TODO comments correspondingly. ### What type of PR is it? Minor ### Questions: * Does the licenses files need update? No * Is there breaking changes for older versions? No * Does this needs documentation? No Author: Kai Jiang <[email protected]> Closes #1556 from vectorijk/minor-remove-comments and squashes the following commits: 1e8ce8a [Kai Jiang] remove TODO comments since 1162 has been solved
### What is this PR for? Since ZEPPELIN-1162 (PR apache#1341) has been solved, we should remove TODO comments correspondingly. ### What type of PR is it? Minor ### Questions: * Does the licenses files need update? No * Is there breaking changes for older versions? No * Does this needs documentation? No Author: Kai Jiang <[email protected]> Closes apache#1556 from vectorijk/minor-remove-comments and squashes the following commits: 1e8ce8a [Kai Jiang] remove TODO comments since 1162 has been solved
### What is this PR for? Since ZEPPELIN-1162 (PR apache#1341) has been solved, we should remove TODO comments correspondingly. ### What type of PR is it? Minor ### Questions: * Does the licenses files need update? No * Is there breaking changes for older versions? No * Does this needs documentation? No Author: Kai Jiang <[email protected]> Closes apache#1556 from vectorijk/minor-remove-comments and squashes the following commits: 1e8ce8a [Kai Jiang] remove TODO comments since 1162 has been solved
### What is this PR for? Since ZEPPELIN-1162 (PR apache#1341) has been solved, we should remove TODO comments correspondingly. ### What type of PR is it? Minor ### Questions: * Does the licenses files need update? No * Is there breaking changes for older versions? No * Does this needs documentation? No Author: Kai Jiang <[email protected]> Closes apache#1556 from vectorijk/minor-remove-comments and squashes the following commits: 1e8ce8a [Kai Jiang] remove TODO comments since 1162 has been solved
What is this PR for?
Some HashSet is declared without types. Fix it in NotebookRestApi
What type of PR is it?
Refactoring
Todos
What is the Jira issue?
https://issues.apache.org/jira/browse/ZEPPELIN-1162
How should this be tested?
All existing test cases related to permissions
Screenshots (if appropriate)
Questions: