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

samples: webusb: Host webusb demo app directly on zephyr doc #62420

Merged
merged 5 commits into from Sep 25, 2023

Conversation

finikorg
Copy link
Collaborator

@finikorg finikorg commented Sep 7, 2023

Move artifacts from private repository to zephyr docs.

Fixes: finikorg/webusb-sample#3

@finikorg finikorg force-pushed the webusp_app branch 2 times, most recently from 1b24599 to 73b3d83 Compare September 8, 2023 10:16
kartben
kartben previously approved these changes Sep 8, 2023
Copy link
Collaborator

@kartben kartben left a comment

Choose a reason for hiding this comment

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

That's awesome - thanks!
Consider my only comment as non-blocking.

Note: the demo page is effectively <html> within <html> which is probably a bit hackish and not really w3c compliant, but I'd say the tradeoff is worthwhile (plus, people have the link to the raw source if needed).

samples/subsys/usb/webusb/README.rst Show resolved Hide resolved
samples/subsys/usb/webusb/README.rst Show resolved Hide resolved
@finikorg finikorg changed the title host webusb demo app directly on zephyr doc samples: webusb: Host webusb demo app directly on zephyr doc Sep 8, 2023
@finikorg finikorg marked this pull request as ready for review September 8, 2023 13:08
kartben
kartben previously requested changes Sep 9, 2023
Copy link
Collaborator

@kartben kartben left a comment

Choose a reason for hiding this comment

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

While I appreciate that you kept commit authorship set to myself for the first commit, it doesn't look right as it probably doesn't compile as-is. Maybe just squash the first two? Or, probably better, re-order with a first commit that moves "your" code to clearly show it's you that's contributing this, and then do the rest with co-authored-by in a second commit.
Thanks again!

finikorg and others added 4 commits September 9, 2023 16:00
Copy file index.html from the external repository
finikorg/webusb-sample.

Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
Include demo.rst to the webusb sample documentation.

Signed-off-by: Benjamin Cabé <benjamin@zephyrproject.org>
Include only README.rst to docs, skipping helpers included with :doc:
directive.

Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
Update webusb README with recent changes after index.html was moved to
sample from the external repository.

Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
tmon-nordic
tmon-nordic previously approved these changes Sep 11, 2023
jfischer-no
jfischer-no previously approved these changes Sep 14, 2023
Copy link
Collaborator

@kartben kartben left a comment

Choose a reason for hiding this comment

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

I spotted a few other issues in the HTML document by putting it through a validator, could you please have a look

samples/subsys/usb/webusb/demo.rst Show resolved Hide resolved
@@ -0,0 +1,125 @@
<html>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just realized it would probably make sense to run this file through a code formatter

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

BTW, you can make commit and push it to this branch after my commits.

samples/subsys/usb/webusb/index.html Outdated Show resolved Hide resolved
samples/subsys/usb/webusb/index.html Outdated Show resolved Hide resolved
samples/subsys/usb/webusb/index.html Outdated Show resolved Hide resolved
samples/subsys/usb/webusb/index.html Outdated Show resolved Hide resolved
samples/subsys/usb/webusb/index.html Outdated Show resolved Hide resolved
samples/subsys/usb/webusb/index.html Outdated Show resolved Hide resolved
samples/subsys/usb/webusb/index.html Show resolved Hide resolved
Running the HTML code through W3C Validator revealed several issues with
the markup that this commit fixes.

Signed-off-by: Benjamin Cabé <benjamin@zephyrproject.org>
Copy link
Collaborator Author

@finikorg finikorg left a comment

Choose a reason for hiding this comment

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

ack @kartben commits

@gmarull gmarull removed their request for review September 19, 2023 07:16
@kartben kartben dismissed their stale review September 23, 2023 23:10

Didn't realize I was blocking this, sorry. Dismissing my own review as I am not in a position to +1 now that I've added commits to this. Luckily it now has enough approvals though :)

@carlescufi carlescufi merged commit c702b03 into zephyrproject-rtos:main Sep 25, 2023
15 checks passed
@finikorg finikorg deleted the webusp_app branch September 25, 2023 08:19
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.

Contribute the web page to Zephyr upstream?
6 participants