-
Notifications
You must be signed in to change notification settings - Fork 0
docs: update readme #49
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
docs: update readme #49
Conversation
📝 WalkthroughWalkthroughThe README.md was rewritten into a comprehensive, structured document: a centered visual header with badges replaces the plain title; a multi‑sentence Introduction and explicit branding notes replace the short description; navigation was reorganized into Installation, Requirements, Usage (including opening, multi‑notebook, read‑only, native integration, and block rendering), Architecture (server/frontend and content interception/translation), Troubleshooting (verification commands and common fixes), Uninstall, Development/Contributing, License, Related projects, and community links. Installation via pip, troubleshooting commands, uninstall instructions, and a contributor footer were added. No source code or public API declarations were changed. Sequence Diagram(s)sequenceDiagram
participant User
participant JLabExt as JupyterLab Extension
participant Server as Deepnote Server Adapter
participant FS as FileSystem
Note over User,JLabExt: User opens a `.deepnote` notebook
User->>JLabExt: Request open `.deepnote` file
JLabExt->>FS: Read `.deepnote` file content
FS-->>JLabExt: Return raw `.deepnote` content
JLabExt->>Server: Intercept & translate to notebook format
alt translation success
Server-->>JLabExt: Notebook JSON
JLabExt->>User: Render notebook in JupyterLab UI
else translation failure
Server-->>JLabExt: Error
JLabExt->>User: Show troubleshooting / unsupported message
end
Possibly related PRs
Suggested reviewers
Pre-merge checks❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #49 +/- ##
=======================================
Coverage 69.56% 69.56%
=======================================
Files 13 13
Lines 253 253
Branches 28 28
=======================================
Hits 176 176
Misses 73 73
Partials 4 4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
9d04873 to
2ef77fd
Compare
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.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
deepnote-dragon.pngis excluded by!**/*.png
📒 Files selected for processing (1)
README.md(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
README.md
1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
14-14: Link fragments should be valid
(MD051, link-fragments)
14-14: Link fragments should be valid
(MD051, link-fragments)
65-65: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
93-93: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
99-99: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
104-104: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
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.
Actionable comments posted: 1
♻️ Duplicate comments (2)
README.md (2)
14-14: Fix typo: "comunity" → "community".CSpell caught this error. The GitHub link text should read "community", not "comunity".
-[Installation](#installation) • [Usage](#usage) • [Development](#development) • [License](#license) • [Deepnote](https://deepnote.com) • [GitHub comunity](https://github.com/deepnote/deepnote) +[Installation](#installation) • [Usage](#usage) • [Development](#development) • [License](#license) • [Deepnote](https://deepnote.com) • [GitHub community](https://github.com/deepnote/deepnote)
1-2: Fix MD041: First line should be a top-level heading.The file opens with an image and comment before any heading, violating MD041. Restructure to place a top-level heading (
#) as the first line, then the image and comment below.- -<!---This is a placeholder for the Deepnote + jupyterlab logo--> +# Deepnote JupyterLab Extension + + +<!---This is a placeholder for the Deepnote + jupyterlab logo-->
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
README.md(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: Build
README.md
[error] 1-1: lint:check failed due to Prettier formatting issues detected in README.md. Run 'prettier --write' to fix code style issues. Step: jlpm run lint:check.
🪛 GitHub Actions: CI
README.md
[error] 14-14: CSpell: Unknown word 'comunity' found. Consider correcting to 'community'.
🪛 markdownlint-cli2 (0.18.1)
README.md
1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
saltenasl
left a comment
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.
Legal risks associated with trademark use of "JupyterLab" of the readme is a blocker for this PR
|
@dinohamzic since you're deeper into the weeds here, can you check if i missed anything? |
|
@Yggdrasill501 can you leave comments in the PR on the why some sections were removed and other heavily rephrased? The PR and related issue don't offer any additional information, and leave us (the reviewers) with making assumptions that may or may not be true. As an alternative, I'd actually suggest we close this PR and tackle each update piece by piece:
|
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.
Actionable comments posted: 2
♻️ Duplicate comments (1)
README.md (1)
33-33: Update Python requirement to 3.10.Prior review feedback (jamesbhobbs) recommended bumping to Python 3.10 or higher. Please update:
- **Python**: 3.9 or higher + **Python**: 3.10 or higher
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
deepnote-jupyterlab-cover-image.pngis excluded by!**/*.png
📒 Files selected for processing (1)
README.md(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
README.md
1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Audit - All
- GitHub Check: Audit - Production
- GitHub Check: Lint & Format
- GitHub Check: build
- GitHub Check: check_release
🔇 Additional comments (1)
README.md (1)
63-88: Clarify rationale for removed content (maintenance policy, backwards compatibility).Prior reviewer (saltenasl) questioned why the maintenance policy and backwards compatibility sections were removed. The current PR lacks explicit explanation for these deletions.
Per dinohamzic's feedback: add PR comments explaining the removals or consider breaking this into incremental PRs (one for logo/badges, one for description, etc.).
@dinohamzic I will clean this up and get it merged, then ask you to review the result (in main) and update the contributing guide #40 for accuracy. The existing contributing guide PR probably needs starting from scratch, this is useable. I will get this merged ASAP. |
jamesbhobbs
left a comment
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.
Header image needs redoing to be trademark compliant (needs the trademark R symbol) thus removed, add it in a new PR @Yggdrasill501
Co-authored-by: Dino Hamzić <dino@subtlebits.com>
af79de7
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.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
README.md(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: check_release
🔇 Additional comments (3)
README.md (3)
13-13: Trademark disclaimer is accurate and comprehensive.The disclaimer on line 13 goes beyond the minimum requirement—it correctly mentions Jupyter, JupyterLab, and Jupyter logos as trademarks of LF Charities, and explicitly states Deepnote's licensed use. This is legally clearer than prior versions.
38-60: Installation and verification sections are clear and well-structured.Requirements are explicit (Python 3.10+, JupyterLab 4.0.0+), pip install is straightforward, and verification commands are provided. This guides users effectively.
80-167: Architecture, Troubleshooting, Compatibility, and Maintenance sections are comprehensive and well-organized.Lines 80–87 clearly explain the extension's architecture (server and frontend components, content provider interception).
Lines 89–126 provide actionable troubleshooting steps and common fixes.
Lines 138–167 detail version compatibility, maintenance policy, and backwards compatibility guarantees—addressing prior feedback to restore these sections. Maintenance policy and pre-release phase are clearly communicated to users.
| # JupyterLab Deepnote | ||
|
|
||
| <div align="center"> | ||
|
|
||
| [](https://github.com/deepnote/jupyterlab-deepnote/actions/workflows/ci.yml) | ||
| [](https://codecov.io/gh/deepnote/jupyterlab-deepnote) | ||
| [](https://pypi.org/project/jupyterlab-deepnote/) | ||
| [](https://pypi.org/project/jupyterlab-deepnote/) | ||
| [](https://opensource.org/licenses/BSD-3-Clause) | ||
|
|
||
| A third-party Deepnote extension for JupyterLab that provides a read‑only viewer for Deepnote `.deepnote` files. | ||
| ## A third-party extension for JupyterLab for viewing Deepnote projects (`.deepnote` files) directly in JupyterLab | ||
|
|
||
| > **Note:** This is an unofficial, community-developed extension and is not affiliated with or endorsed by Project Jupyter. JupyterLab is a trademark of LF Charities, Inc. | ||
| > **Note:** This is an unofficial, community-developed extension and is not affiliated with or endorsed by Project Jupyter. Jupyter, JupyterLab and the Jupyter logos are trademarks or registered trademarks of LF Charities, used by Deepnote Inc with permission. | ||
| ## Requirements | ||
| [Installation](#installation) • [Usage](#usage) • [Development](#development) • [License](#license) • [Deepnote](https://deepnote.com) • [GitHub community](https://github.com/deepnote/deepnote) | ||
|
|
||
| - Python 3.9 or higher | ||
| - JupyterLab >= 4.4.0, < 5.0.0 | ||
| </div> | ||
|
|
||
| ## Install | ||
| --- |
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.
🧩 Analysis chain
Verify image element is present and properly sized in the header.
The header structure looks good—heading on line 1, badges, disclaimer, and navigation all properly placed. However, the provided code diff does not show an <img> tag in the centered div (lines 3–17). Past feedback indicated the image was replaced and resized. Either the image is missing from the diff view or it was inadvertently removed.
Verify the image element is present and confirm it has a max-width style constraint as suggested in prior feedback:
If the image is missing, it should be restored inside the centered <div> with appropriate width constraints.
🏁 Script executed:
#!/bin/bash
# Check if README.md contains an img tag in the header section (first 50 lines)
head -50 README.md | grep -i '<img'
echo "---"
# Also look for any image reference in the full file
grep -i '<img' README.mdLength of output: 115
🏁 Script executed:
#!/bin/bash
# Search for img tags in README.md (any location)
echo "=== Checking for <img> tags in README.md ==="
rg -n '<img' README.md || echo "No <img> tags found"
echo ""
echo "=== First 50 lines of README.md ==="
head -50 README.mdLength of output: 2727
🏁 Script executed:
#!/bin/bash
# Search for img tags in README.md across full file
echo "=== Searching for <img> tags in README.md ==="
rg -n '<img' README.md || echo "No <img> tags found"
echo ""
echo "=== Displaying first 25 lines of README.md ==="
head -25 README.md
echo ""
echo "=== Checking header section (lines 1-20) for any image-related content ==="
head -20 README.md | grep -i 'image\|img\|src\|logo' || echo "No image-related content found in header"Length of output: 2647
Add missing image element to header.
The header currently has no image element. Every <img> tag search across the file returns nothing. The centered div (lines 3–17) contains only badges and text. If an image was supposed to be restored per prior feedback with a max-width constraint, it needs to be added to the header section.
🤖 Prompt for AI Agents
In README.md around lines 1 to 19, the header is missing the intended image
element; add an <img> tag inside the centered div (above or beside the badges)
with appropriate alt text and a source (relative path or URL), and apply the
max-width constraint (e.g., style="max-width: 320px;" or a CSS class) so the
image scales correctly; ensure the image tag is HTML-safe for GitHub README and
keep it within the existing centered div so it displays in the header.
Summary by CodeRabbit