Skip to content

Conversation

@kaburagisec
Copy link
Contributor

No description provided.

@kaburagisec kaburagisec requested a review from Copilot November 4, 2025 10:54
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR bumps the version from 0.2.4 to 0.2.5 and introduces a new --output feature for the CLI that allows saving command outputs to files in various formats (JSON, XML, plain text), while also cleaning up unused exports and fixing string formatting issues.

Key changes:

  • Added --output CLI argument to save command results to JSON, XML, or plain text files with automatic format detection
  • Removed ONVIFOperator from the public API exports in __init__.py
  • Fixed f-string usage where f-strings were unnecessary (changed to regular strings)

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
pyproject.toml Version bump to 0.2.5
onvif/cli/main.py Added --output feature with format detection, new helper functions for serialization, and validation logic
onvif/cli/interactive.py Updated version string in ASCII art banner
onvif/init.py Removed ONVIFOperator from public exports
examples/logger.py Fixed unnecessary f-string usage
examples/error_handling.py Fixed unnecessary f-string usage in multiple locations
examples/add_media_profile.py Fixed unnecessary f-string usage
README_ID.md Updated version badges, improved library philosophy section, added output examples
README.md Updated version badges, improved library philosophy section, added output examples
CONTRIBUTING.md Updated contribution guidelines to target dev branch and improved setup instructions

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

# Save output to file if specified
if args.output:
# Auto-enable debug mode for XML output
effective_debug = args.debug or args.output.lower().endswith(".xml")
Copy link

Copilot AI Nov 4, 2025

Choose a reason for hiding this comment

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

The debug mode calculation is duplicated here and at line 281-283 (as auto_debug). Consider using the auto_debug variable directly instead of recalculating effective_debug to avoid code duplication. You could pass auto_debug to the function instead of recalculating.

Suggested change
effective_debug = args.debug or args.output.lower().endswith(".xml")
effective_debug = auto_debug

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

No need, because debugging is needed because the desired output file is XML.

Comment on lines 388 to 390
output_data = {}

if ext == ".json":
Copy link

Copilot AI Nov 4, 2025

Choose a reason for hiding this comment

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

The output_data dictionary is initialized but only used in the JSON branch (line 392-405). Move this initialization inside the if ext == \".json\": block to improve code clarity and avoid unnecessary variable initialization.

Suggested change
output_data = {}
if ext == ".json":
if ext == ".json":
output_data = {}

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, let's use this change.

@kaburagisec kaburagisec merged commit 5a62bcb into main Nov 4, 2025
5 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.

2 participants