[nteract/web] Save/upload to Github and other improvements. #5240
Conversation
ramantehlan
changed the title
| if (!forkFound) | ||
| createFork(org, repo) | ||
| }) | ||
| const getFileType = (fileName: string) => { |
There was a problem hiding this comment.
This function is not complete, but it will be used to get file type and update the mode/language type in the editor.
| onChange: handleChange | ||
| } | ||
| } | ||
|
|
||
|
|
||
| const listForks = (owner, repo) => { |
There was a problem hiding this comment.
These functions will be moved to a separate util folder since they all are used to work with Github API and are not part of the main component.
|
This pull request introduces 1 alert when merging c7039c6 into f089b66 - view on LGTM.com new alerts:
|
There was a problem hiding this comment.
Thanks @ramantehlan. I've made a few comments prior to @captainsafia's review.
| @@ -302,7 +487,7 @@ function addBuffer(e){ | |||
| } | |||
|
|
|||
|
|
|||
| // We won't be following this logic, we will render the data from github and only send changes to binder | |||
| // We won't be following this logic, we will render the data from github and only send changes to binder | |||
There was a problem hiding this comment.
Let's go ahead and remove the code if it is not being used. Thanks.
There was a problem hiding this comment.
It will be used in the next step when will connect binder to nteract web/play. However, that comment is irrelevant now, so I will update it.
Replace all placeholder console with notificationThis comment was generated by todo based on a
|
Make the below mode dynamic by identifying the language on openThis comment was generated by todo based on a
|
|
This pull request introduces 1 alert when merging b1cb93a into f089b66 - view on LGTM.com new alerts:
|
|
This pull request introduces 1 alert when merging 3c6647d into f089b66 - view on LGTM.com new alerts:
|
|
This pull request introduces 2 alerts when merging 7044197 into f089b66 - view on LGTM.com new alerts:
|
|
@captainsafia and @willingc, Thank you for the review, I have addressed the comments and this PR is ready for review again. : ) I also wanna point out that I have created just one TODO for all the replacements for console notification with actual notification. |
This PR is to save back changes to the repo or the fork.
It will also make the following changes: