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

Implement Issue Config #20956

Merged
merged 36 commits into from Mar 28, 2023
Merged

Implement Issue Config #20956

merged 36 commits into from Mar 28, 2023

Conversation

JakobDev
Copy link
Contributor

Closes #20955

This PR adds the possibility to disable blank Issues, when the Repo has templates. This can be done by creating the file .gitea/issue_config.yaml with the content blank_issues_enabled in the Repo.

I have no experience in Go, so this code might not be perfect, but it is working. If anybody knows how to do that better, just say it. It is also missing documentation at the moment.

@JakobDev JakobDev mentioned this pull request Aug 29, 2022
8 tasks
@lunny
Copy link
Member

lunny commented Aug 29, 2022

should the path be .gitea/issue_template/config.yaml and to keep compatible with Github to support .github/issue_template/config.yaml

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Aug 29, 2022
@JakobDev
Copy link
Contributor Author

@lunny I have now changed the paths

@lunny lunny added the type/enhancement An improvement of existing functionality label Nov 23, 2022
@JakobDev JakobDev mentioned this pull request Dec 1, 2022
@JakobDev
Copy link
Contributor Author

@lunny Any more suggestions or should I add documentation, so this can be merged?

@lunny
Copy link
Member

lunny commented Dec 13, 2022

Hi, sorry to be late to reply. Maybe you could add some documentation and tests? And maybe @wolfogre could also have a review because he contributed some related PRs.

@JakobDev
Copy link
Contributor Author

The documentation has now been added, but i don't know, how to write tests for this.

modules/context/repo.go Outdated Show resolved Hide resolved
routers/web/repo/issue.go Outdated Show resolved Hide resolved

### Possible Options

| Key | Description | Type |
Copy link
Member

@delvh delvh Dec 13, 2022

Choose a reason for hiding this comment

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

I think Default is also a good idea for this table.

templates/repo/issue/choose.tmpl Outdated Show resolved Hide resolved
modules/context/repo.go Outdated Show resolved Hide resolved
modules/context/repo.go Outdated Show resolved Hide resolved
modules/context/repo.go Outdated Show resolved Hide resolved
modules/context/repo.go Outdated Show resolved Hide resolved
modules/context/repo.go Show resolved Hide resolved
@JakobDev
Copy link
Contributor Author

I had now done a bit of code refactoring:

  • I now use yaml.v3
  • The list with the filenames is shorten
  • If a Issue config contains a error, it is now shown while viewing the file

ctx.Flash.Error() requires loading the config every time you visit the main page of the repo (which is a waste of resources), or else it will just be shown after visiting, the Issue template list, which is confusing, so I decided to stick with the current solution.

@JakobDev JakobDev requested review from lunny, wolfogre and delvh and removed request for lunny and wolfogre December 20, 2022 14:10
routers/web/repo/issue.go Outdated Show resolved Hide resolved
templates/repo/issue/choose.tmpl Outdated Show resolved Hide resolved
modules/context/repo.go Show resolved Hide resolved
@GiteaBot GiteaBot added the lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. label Feb 16, 2023
@lunny
Copy link
Member

lunny commented Feb 16, 2023

Need @delvh approval.

@JakobDev JakobDev mentioned this pull request Mar 3, 2023
@JakobDev
Copy link
Contributor Author

JakobDev commented Mar 3, 2023

Ping @delvh

@wxiaoguang
Copy link
Contributor

Happened to see this PR ..... maybe the maintainer team need some mechanisms to handle the "pending reviews"?

Should I help to call the TOC (again)?

@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Mar 15, 2023
@lunny lunny added this to the 1.20.0 milestone Mar 15, 2023
@lunny
Copy link
Member

lunny commented Mar 15, 2023

Need @delvh 's review.

Copy link
Member

@delvh delvh left a comment

Choose a reason for hiding this comment

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

Yep, sorry for the late reply, my TODO list is way too long already.
Furthermore, I'm still not quite a fan of using a boolean where I think a string is more fitting.
However, I can at least accept this PR assuming everyone knows that this is probably not the best option for a long-term solution.

modules/context/repo.go Show resolved Hide resolved
modules/structs/issue.go Outdated Show resolved Hide resolved
modules/structs/issue.go Show resolved Hide resolved
@lunny lunny removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Mar 15, 2023
@delvh delvh self-requested a review March 15, 2023 20:41
@codecov-commenter

This comment was marked as off-topic.

Copy link
Member

@delvh delvh left a comment

Choose a reason for hiding this comment

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

I've now tested the functionality.
A few closing remarks:

  • I think the API route can be either under repo or under issue. I can certainly see why it's currently under repo. GitHub doesn't offer this API at the moment, so I'm fine with both
  • I am not a fan of having _ in URLs but seeing that other URLs also have the _ already, I'll allow it
  • There should be a divider between the issue templates and the contact links, however, this can be postponed for a later PR:
    image

I'm still not a fan of using a boolean for that struct, but other than that I don't see anything holding this PR back.

@JakobDev
Copy link
Contributor Author

There should be a divider between the issue templates and the contact links, however, this can be postponed for a later PR:

I personally prefer it when it has no divider and everything looks the same (except for the buttons) in the box.

I'm still not a fan of using a boolean for that struct, but other than that I don't see anything holding this PR back.

I still think, it's better to add a extra setting. A string that is a boolean is confusing and can cause problems with the API.

@delvh delvh added reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. type/feature Completely new functionality. Can only be merged if feature freeze is not active. and removed type/enhancement An improvement of existing functionality labels Mar 28, 2023
@jolheiser jolheiser enabled auto-merge (squash) March 28, 2023 17:57
@jolheiser
Copy link
Member

🎺 🤖

@jolheiser jolheiser merged commit f384b13 into go-gitea:main Mar 28, 2023
2 checks passed
@jolheiser jolheiser removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Mar 28, 2023
@JakobDev JakobDev deleted the noblankissues branch March 28, 2023 18:24
zjjhot added a commit to zjjhot/gitea that referenced this pull request Mar 29, 2023
* upstream/main:
  Refactor internal API for git commands, use meaningful messages instead of "Internal Server Error" (go-gitea#23687)
  Add CSS rules for basic colored labels (go-gitea#23774)
  Add meilisearch support (go-gitea#23136)
  Add missing translation for `actions.runners.reset_registration_token_success` (go-gitea#23732)
  [skip ci] Updated translations via Crowdin
  Implement Issue Config (go-gitea#20956)
  Set repository link based on the url in package.json for npm packages (go-gitea#20379)
  Add API to manage issue dependencies (go-gitea#17935)
  Add creation time in tag list page (go-gitea#23693)
  Make minio package support legacy MD5 checksum (go-gitea#23768)
  Yarden Shoham has a new email address (go-gitea#23767)
  fix br display for packages curls (go-gitea#23737)
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/feature Completely new functionality. Can only be merged if feature freeze is not active.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Disable blank Issues
8 participants