Skip to content
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

chore: add jupyter to supported online IDEs #186

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

jvelezpo
Copy link
Contributor

@jvelezpo jvelezpo commented Apr 12, 2023

Pre-PR Checklist

  • I have run npm test locally and there are no warnings or errors, from lint or test
  • I have added screenshots if applicable

Summary

  • Adds Jupyter to the list of supported online IDEs
  • Adds Try Jupyter as well

There is an issue with the current implementation and it is that I could not find the concept of project while working with it, so this PR sends a heartbeat the name of the current file/notebook as project, if the user navigates into multiple files in its session in Jupyter it will create a very long list of projects in its dashboard, which is not ideal IMO (see screenshot bellow) @alanhamlett maybe do you have more expertise with Jupyter and will know how to get a correct value to send as project on each heartbeat?

Screenshots

image

Issue

#82

Reviewers

@jvelezpo jvelezpo requested a review from alanhamlett April 17, 2023 15:33
@alanhamlett
Copy link
Member

alanhamlett commented Apr 17, 2023

Let's send "project":"<<LAST_PROJECT>>" same as other domains for Jupyter. I think that's better than using filename as project, since like you said it would create too many projects.

We should however, send the file name as entity in the heartbeat instead of using the domain/url. Then we should also change "category":"coding" and append jupyter-browser to the User-Agent string.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants