Skip to content

Added Description Field for Secrets and Variables #33526

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

Merged
merged 21 commits into from
Mar 17, 2025
Merged

Conversation

LaoQi
Copy link
Contributor

@LaoQi LaoQi commented Feb 6, 2025

Fixes #33484

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Feb 6, 2025
@github-actions github-actions bot added modifies/translation modifies/api This PR adds API routes or modifies them modifies/go Pull requests that update Go code modifies/templates This PR modifies the template files modifies/migrations labels Feb 6, 2025
@lunny lunny added this to the 1.24.0 milestone Feb 6, 2025
@LaoQi
Copy link
Contributor Author

LaoQi commented Feb 8, 2025

Is there a standardized length limit for these texts in the project? Maybe we should define constants for these. For now, I've capped them all at 4096 characters

@lunny
Copy link
Member

lunny commented Feb 9, 2025

LGTM except #33526 (comment)

Screenshots are necessary.

@LaoQi
Copy link
Contributor Author

LaoQi commented Feb 9, 2025

I noticed that the data field also doesn't have a length restriction. I'll add a length constraint to it as well. For this field, I think a limit of 65536 would be appropriate, although we can truncate the display when showing it.
The input data is generally UTF-8 encoded, so I will use UTF-8 character counting.
eg:

const (
	VariableDataMaxLength        = 65536
	VariableDescriptionMaxLength = 4096
)
if utf8.RuneCountInString(data) > VariableDataMaxLength {
	data = string([]rune(data)[:VariableDataMaxLength])
}

if utf8.RuneCountInString(description) > VariableDescriptionMaxLength {
	description = string([]rune(description)[:VariableDescriptionMaxLength])
}

Any other suggestions?

@LaoQi
Copy link
Contributor Author

LaoQi commented Feb 9, 2025

034215
034046
034021

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@lunny
Copy link
Member

lunny commented Feb 9, 2025

I think the description should be moved under value because it's optional. And it's better to have 2 rows at least.

@LaoQi
Copy link
Contributor Author

LaoQi commented Feb 10, 2025

I think the description should be moved under value because it's optional. And it's better to have 2 rows at least.

Does it refer only to the form, or does it also include the list? Perhaps it would be better to place the description in the list above.

@LaoQi
Copy link
Contributor Author

LaoQi commented Feb 10, 2025

225010

@silverwind
Copy link
Member

Were all reviews addressed? If so, please resolve them.

@LaoQi
Copy link
Contributor Author

LaoQi commented Mar 16, 2025

All problems have been fixed.

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Mar 16, 2025
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Mar 17, 2025
@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Mar 17, 2025
@lunny lunny enabled auto-merge (squash) March 17, 2025 19:04
@lunny lunny merged commit 8f051d5 into go-gitea:main Mar 17, 2025
26 checks passed
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Mar 17, 2025
zjjhot added a commit to zjjhot/gitea that referenced this pull request Mar 19, 2025
* giteaofficial/main:
  [skip ci] Updated translations via Crowdin
  Optimize Layout Styles of Filelist (go-gitea#33920)
  [skip ci] Updated translations via Crowdin
  update go version for devcontainers (go-gitea#33923)
  Added Description Field for Secrets and Variables  (go-gitea#33526)
  Try to figure out attribute checker problem (go-gitea#33901)
  Defer captcha script loading (go-gitea#33919)
  Fix file tree issues (go-gitea#33916)
  Remove unused or abused styles (go-gitea#33918)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. modifies/api This PR adds API routes or modifies them modifies/go Pull requests that update Go code modifies/migrations modifies/templates This PR modifies the template files modifies/translation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Description Field for Secrets and Variables
7 participants