-
-
Notifications
You must be signed in to change notification settings - Fork 6.2k
Added Description Field for Secrets and Variables #33524
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
Conversation
techknowlogick
left a comment
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.
Thanks for this PR!
A few tiny things that did it so far.
| </div> | ||
| <div class="field"> | ||
| <label for="secret-description">{{ctx.Locale.Tr "secrets.creation.description"}}</label> | ||
| <input autofocus |
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.
Probably best to leave autofocus off of this field as the others already
| @@ -0,0 +1,20 @@ | |||
| // Copyright 2024 The Gitea Authors. All rights reserved. | |||
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.
2025
| </div> | ||
| <div class="field"> | ||
| <label for="dialog-variable-description">{{ctx.Locale.Tr "secrets.creation.description"}}</label> | ||
| <input autofocus |
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.
Same as above
| Name: "description", | ||
| Description: "some description", | ||
| ExpectedStatus: http.StatusCreated, | ||
| }, |
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.
Could you add a test that leaves out the description field, just to ensure that it'll fall back to the default?
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.
These tests only check if descriptions can be written. In reality, the default tests written at api_repo_secrets_test.go#L33 are cases without descriptions. I haven’t thought of any other test cases yet. Any suggestions?
|
@LaoQi i saw you closed this, was that intentional? |
|
There too many lint error on it. After re-linting the code, I will submit a new pull request. |
Basic support for #33484