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

Update generate_openapi_code.sh #615

Closed
wants to merge 1 commit into from

Conversation

RAVI-PRAVEEN
Copy link

@RAVI-PRAVEEN RAVI-PRAVEEN commented Oct 8, 2024

Key Changes:
Modular Functions: The script is now organized into functions for logging actions, compiling TypeSpec, and running Poetry tasks. Logging: Introduced log_action for logging actions and errors with timestamps. This helps track what the script is doing in real-time. Error Handling: Added || { ... } constructs to gracefully handle errors and log them. Optional Poetry Update: The --update-poetry flag is available to update Poetry dependencies before running tasks. If not passed, Poetry runs without updating. Execution Time: Logs the total time taken for the script to execute, making it easier to track performance.


Important

Refactor generate_openapi_code.sh to modularize functions, add logging, improve error handling, and introduce optional Poetry update flag.

  • Modularization:
    • Split script into functions: log_action, compile_typespec, run_poetry_tasks, and usage.
  • Logging:
    • Added log_action() to log actions with timestamps to script.log.
    • Logs start and end time of script execution.
  • Error Handling:
    • Added || { ... } constructs for error handling in compile_typespec and run_poetry_tasks.
  • Optional Poetry Update:
    • Introduced --update-poetry flag to optionally update Poetry dependencies.
  • Misc:
    • Added usage instructions for script options.

This description was created by Ellipsis for 543f189. It will automatically update as commits are pushed.

Key Changes:
Modular Functions: The script is now organized into functions for logging actions, compiling TypeSpec, and running Poetry tasks.
Logging: Introduced log_action for logging actions and errors with timestamps. This helps track what the script is doing in real-time.
Error Handling: Added || { ... } constructs to gracefully handle errors and log them.
Optional Poetry Update: The --update-poetry flag is available to update Poetry dependencies before running tasks. If not passed, Poetry runs without updating.
Execution Time: Logs the total time taken for the script to execute, making it easier to track performance.
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Reviewed everything up to 543f189 in 24 seconds

More details
  • Looked at 104 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. scripts/generate_openapi_code.sh:4
  • Draft comment:
    The 'set -xe' command is redundant because 'set -e' is already used, and 'set -x' is not necessary for logging purposes since the script already logs actions. Consider removing 'set -xe'.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable:
    The comment is addressing a change in the script where 'set -xe' is used. The script does have logging, which might reduce the need for 'set -x', but 'set -xe' is often used together for error checking and debugging. The comment is suggesting a code change, which aligns with the rules for keeping comments.
    The comment assumes that logging completely replaces the need for 'set -x', which might not be true if detailed command tracing is needed for debugging. The comment might be too quick to suggest removal without considering the debugging benefits of 'set -x'.
    While logging provides some visibility, 'set -x' offers a different level of detail that can be useful. However, if the logging is deemed sufficient, the comment's suggestion could be valid.
    The comment is about a change made in the diff and suggests a potential code improvement. It should be kept as it proposes a specific action to consider.

Workflow ID: wflow_KACHeq9QRgXodOwm


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@creatorrr creatorrr mentioned this pull request Oct 8, 2024
@creatorrr
Copy link
Contributor

Have you verified if the bash script works correctly? Please post output here that it does and I'd be happy to merge this pr

@creatorrr
Copy link
Contributor

Duplicate of #547

@creatorrr creatorrr marked this as a duplicate of #547 Oct 10, 2024
@creatorrr creatorrr closed this Oct 10, 2024
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