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 action id prefix to act #1542

Merged
merged 1 commit into from
Jan 13, 2025
Merged

Conversation

wintonzheng
Copy link
Contributor

@wintonzheng wintonzheng commented Jan 13, 2025

Important

Update ACTION_PREFIX to act and reorder prefix constants in id.py.

  • Behavior:
    • Update ACTION_PREFIX from a to act in skyvern/forge/sdk/db/id.py.
    • generate_action_id() now uses act prefix.
  • Misc:
    • Reorder prefix constants in id.py for better organization.

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

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 da57d29 in 12 seconds

More details
  • Looked at 63 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. skyvern/forge/sdk/db/id.py:152
  • Draft comment:
    The prefix for action IDs has been updated from 'a' to 'act'. Ensure that any external systems or documentation referencing the old prefix are updated accordingly.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The PR changes the prefix for action IDs from 'a' to 'act'. The change is consistent with the naming convention used for other prefixes, which are more descriptive. The change is correctly implemented in the generate_action_id function.

Workflow ID: wflow_zJVjkkjQW4IqaYbp


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

TASK_PREFIX = "tsk"
USER_PREFIX = "u"
STEP_PREFIX = "stp"
ACTION_PREFIX = "act"
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this gonna cause any regression issues?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i don't think so

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 hard code to detect the id pattern. it's just a visual thing for human

@wintonzheng wintonzheng merged commit 52392fa into main Jan 13, 2025
6 checks passed
@wintonzheng wintonzheng deleted the shu/update_action_id_prefix branch January 13, 2025 17:03
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