Skip to content

Conversation

@Vincamine
Copy link
Contributor

Notes for Reviewers

This PR fixes #715

Signed commits

  • Yes, I signed my commits.

Signed-off-by: Wyllie Fang <wylliefang@gmail.com>
@netlify
Copy link

netlify bot commented Jul 30, 2025

Deploy Preview for bejewelled-pegasus-b0ce81 ready!

Name Link
🔨 Latest commit 0493a85
🔍 Latest deploy log https://app.netlify.com/projects/bejewelled-pegasus-b0ce81/deploys/688a7f188b919b0007a856db
😎 Deploy Preview https://deploy-preview-724--bejewelled-pegasus-b0ce81.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Copy link
Contributor

@zihanKuang zihanKuang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image image

**Recommended Approach (Page Bundling):**
```markdown
// Directory structure
content/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since a detailed structural tree has already been provided, a brief textual explanation here will suffice.

### How to Add a Video
```mermaid
graph LR
A[Add Video] --> B{Size >50MB?}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

While the diagram is clear, its presentation isn't optimal. Since it's essentially a two-step logic, let's switch to a different format. We can just use a "best practice" statement here.

- **Cloudflare Stream** (Paid, enterprise-grade)
- **Vimeo** (Paid, professional features)

#### Migration Guide
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no need to mention "migration" here. This guide is for first-time learning path creation, and we no longer recommend using the static folder. Therefore, simply omit any reference to the usestatic method.

D --> F[Local Reference]
```

#### Best Practices for Video Storage
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can directly introduce the two methods here, marking one as (Recommended).

As for "best practices," you can include them at the end, alongside your "Critical Considerations," changing that heading to "Best Practices."

Signed-off-by: Wyllie Fang <wylliefang@gmail.com>
@Vincamine Vincamine requested a review from zihanKuang July 30, 2025 20:23
@Vincamine
Copy link
Contributor Author

Update:

  1. Remove 'how to add video graph' since it's just a 2-way option
  2. Remove // Directory structure
  3. Remove migration guide part
  4. Introduce 2 methods directly with one marked as recommended.
  5. Add 'best practice' in critical considerations

Copy link
Contributor

@zihanKuang zihanKuang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@zihanKuang zihanKuang merged commit cb342fa into layer5io:master Jul 30, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Docs] Enhance Documentation: Guidelines for External Media and Video Hosting

2 participants