Skip to content

Conversation

@tyler-romero
Copy link
Contributor

@tyler-romero tyler-romero commented Nov 30, 2024

How to Review:

This is a large documentation PR with lots of changes to formatting, wording, and site organization. In order to effectively review, please check out this branch (tyler/more-code-samples) and run make develop-docs-comprehensive to see a development version of the site.

Changes (selected)

  1. Update documentation to reflect that we support Python 3.9+
  2. Document usage of ask_ml and ask_confident
  3. Reorganize site to improve consistency and flow (most significantly, docs for developing applications are separate from docs detailing sample applications)
  4. Document use of framegrab and tuning of motion detection
  5. Document Alerts, update documentation for the edge-endpoint and groundlight/stream

@tyler-romero
Copy link
Contributor Author

Preview of the changes / reorganization
image

Copy link
Collaborator

@brandon-wada brandon-wada left a comment

Choose a reason for hiding this comment

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

Yes and-
#280
Github won't let me recommend changes on lines that aren't near direct changes that you've made

Overall looks great and is a much needed improvement. I'd check with Jared before committing to dark mode, it's affecting the color scheme quite a bit.

#"3.6", # Default on Ubuntu18.04 but openapi-generator fails
# "3.7", # Removed support as of 0.17
# "3.8", # Removed support as of 0.19
# "3.9", # Removed support as of 0.19
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wait, this should be 3.8. We still support 3.9

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah my bad! replace error

// image is expected to be a "social card". Logo for now.
image: "img/gl-icon400.png",
colorMode: {
defaultMode: 'dark',
Copy link
Collaborator

Choose a reason for hiding this comment

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

While I'm also a fan of dark mode, the landing page /python-sdk/ looks bad with dark mode on. The backgrounds on the images no longer blend in and I think the blue changes away from our normal Groundlight blue

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah this was maybe a bit premature - we're actually moving to a custom (dark) landing page in this PR: #278

Couple nits I noticed while reviewing from parts of the doc that weren't
touched by the original set of changes in tyler/more-code-samples
@tyler-romero
Copy link
Contributor Author

Thanks brandon! merged your changes into this branch

@tyler-romero tyler-romero marked this pull request as ready for review December 2, 2024 22:39
@tyler-romero
Copy link
Contributor Author

There are lots of line changes for sure, but the reason there are tens of thousands is due to the node package updates in package-lock.json

Copy link
Contributor

@paulina-positronix paulina-positronix left a comment

Choose a reason for hiding this comment

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

LGTM thank you, this is great work! I have some suggestions primarily around the order of ask_ method and labeling ask_ml as advanced.

@@ -1,15 +1,15 @@
# A Quick Example: Live Stream Alert
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be removed or replaced by your new way of processing streams described in Low-Code Stream Processor?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done!

Tyler Romero and others added 2 commits December 3, 2024 11:03
Co-authored-by: Paulina Varshavskaya <88207457+paulina-positronix@users.noreply.github.com>
@tyler-romero tyler-romero merged commit 5e9da70 into main Dec 3, 2024
8 checks passed
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.

4 participants