-
Notifications
You must be signed in to change notification settings - Fork 590
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
Merging main to develop #5250
Merging main to develop #5250
Conversation
WalkthroughThe changes in this pull request focus on updating the documentation for FiftyOne, specifically regarding the integration with CVAT. A new environment variable, Changes
Possibly related PRs
Suggested labels
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
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: 0
🧹 Outside diff range and nitpick comments (1)
docs/source/integrations/cvat.rst (1)
Line range hint
221-237
: LGTM! Consider adding more context about email requirementThe addition of the
FIFTYONE_CVAT_EMAIL
environment variable is well-documented and follows the existing pattern. However, it would be helpful to add a brief note explaining when/why this email field is required for authentication.Add a brief explanation like:
export FIFTYONE_CVAT_EMAIL=... # if applicable + # Note: The email field is required when using CVAT's SSO authentication
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (6)
docs/source/integrations/cvat.rst
(2 hunks)docs/source/plugins/developing_plugins.rst
(5 hunks)docs/source/plugins/using_plugins.rst
(4 hunks)docs/source/release-notes.rst
(2 hunks)docs/source/teams/secrets.rst
(2 hunks)docs/source/user_guide/annotation.rst
(1 hunks)
🔇 Additional comments (9)
docs/source/teams/secrets.rst (1)
70-70
: LGTM! The new secret key is properly documented.
The addition of FIFTYONE_CVAT_EMAIL
is consistent across both the YAML configuration and Python usage examples, maintaining proper documentation standards.
Let's verify the consistency of this secret across the codebase:
Also applies to: 87-87
✅ Verification successful
Secret key FIFTYONE_CVAT_EMAIL
is consistently documented and properly used across the codebase
The verification shows that the secret is consistently documented and used across multiple documentation files:
- Properly listed in configuration examples in
teams/secrets.rst
- Correctly demonstrated in usage examples with
ctx.secrets["FIFTYONE_CVAT_EMAIL"]
- Consistently documented in integration guides and tutorials
- Maintains the same format and naming convention throughout the codebase
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the FIFTYONE_CVAT_EMAIL secret is consistently documented
# and implemented across the codebase
# Check for the secret in configuration files and documentation
rg -A 2 "FIFTYONE_CVAT_EMAIL"
# Check for any potential usages in Python code
ast-grep --pattern 'ctx.secrets["FIFTYONE_CVAT_EMAIL"]'
Length of output: 3115
docs/source/plugins/using_plugins.rst (1)
299-299
: LGTM! The new secret key is comprehensively documented.
The FIFTYONE_CVAT_EMAIL
secret is consistently documented across all relevant sections:
- Plugin info output
- YAML configuration
- Environment variables setup
- Python usage example
The documentation maintains proper formatting and provides clear guidance for users.
Also applies to: 473-473, 496-496, 509-509
docs/source/user_guide/annotation.rst (1)
360-361
: LGTM! The email configuration is properly documented.
The addition of the FIFTYONE_CVAT_EMAIL
environment variable is well-documented with:
- Clear indication that it's optional ("if applicable")
- Proper integration into the environment variables section
- Consistent formatting with other configuration options
Also applies to: 368-368
docs/source/integrations/cvat.rst (2)
Line range hint 1532-1549
: LGTM! Secret declaration is properly documented
The addition of FIFTYONE_CVAT_EMAIL
to the plugin's secrets list is consistent with the environment variable changes and follows the established pattern.
Line range hint 1562-1563
: LGTM! Code example properly demonstrates secret usage
The example correctly shows how to access the new FIFTYONE_CVAT_EMAIL
secret in operators, maintaining consistency with the documentation above.
docs/source/plugins/developing_plugins.rst (2)
255-255
: LGTM! Secret is properly included in plugin example
The addition of FIFTYONE_CVAT_EMAIL
to the example fiftyone.yml file is consistent with the CVAT integration changes.
2508-2508
: LGTM! Code example properly demonstrates secret usage in panels
The example correctly shows how to access the new FIFTYONE_CVAT_EMAIL
secret in panels, maintaining consistency with the plugin documentation.
docs/source/release-notes.rst (2)
54-55
: LGTM: Added referrerPolicy for reverse proxy support
The addition of referrerPolicy
improves the App's functionality when running behind reverse proxies.
66-67
: LGTM: Optimized object detection evaluation
The optimization of object detection evaluation using r-trees improves performance.
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.
🔩
Summary by CodeRabbit
New Features
FIFTYONE_CVAT_EMAIL
environment variable for CVAT server configuration.Documentation
FIFTYONE_CVAT_EMAIL
secret.