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

TC actions name as composite id namespace/id #1159

Merged
merged 2 commits into from
Sep 25, 2024
Merged

Conversation

kesarevs
Copy link
Collaborator

No description provided.

const val NAME = "id"
const val DESCRIPTION = "the second part of the composite `${ActionName.NAME}` field"
const val MIN_LENGTH = 5
const val MAX_LENGTH = 30
Copy link
Collaborator

Choose a reason for hiding this comment

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

I was thinking of raising the max length for the action id, maybe to 50 symbols. 30 seems a bit too short for the action name, but should be ok for the namespace name. Do you have any constraints on plugin id length on the Marketplace side?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The limit is 190 characters (for TC Actions it will be the limit for the composite id namespace/name).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Let's discuss it separately since there are no actual users right now. We will be able to extend this limit anytime in the future.

@boris-yakhno
Copy link
Collaborator

I've implemented the suggested fixes in my fork – boris-yakhno@eeffeaf
Feel free to use some or all of the changes from my commit.

* Composite name format validation
* Name validation changes
* Tests
@boris-yakhno
Copy link
Collaborator

Added a commit with fixes to the PR, as suggested by @kesarevs

@kesarevs kesarevs merged commit 2dbc2e2 into master Sep 25, 2024
2 checks passed
@kesarevs kesarevs deleted the tc-action-namespaces branch September 25, 2024 10:07
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.

3 participants