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.

Sign up for GitHub

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

Jump to bottom

WIP: add priority to issues #20959

Closed
wants to merge 13 commits into from
Closed

WIP: add priority to issues #20959

wants to merge 13 commits into from

Conversation

techknowlogick
Copy link
Member

No description provided.

@techknowlogick techknowlogick added the type/feature Completely new functionality. Can only be merged if feature freeze is not active. label Aug 25, 2022
@techknowlogick techknowlogick added this to the 1.18.0 milestone Aug 25, 2022
@techknowlogick techknowlogick changed the title add priority to issues WIP: add priority to issues Aug 25, 2022
@lafriks
Copy link
Member

lafriks commented Aug 25, 2022

I think this conflicts with my PR #11669

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

@lafriks

I think this conflicts with my PR #11669

It does, but this is a different approach. This will let users 1. customize the weight of the priority 2. ensure only one priority per issue, and 3. sorting issues by weight of priority

@lafriks
Copy link
Member

lafriks commented Aug 26, 2022 •
edited
Loading

Mine is doing pretty much the same except that it has predefined set of weights. In the end in mine issues will also have priority weight that will be calculated and saved to issue only based on issues label max priority weight that will allow sorting issues by priority.
It could be used in the end with option that is also requested in gitea issues to allow define and assign only single label per group

@lunny
Copy link
Member

lunny commented Aug 27, 2022

Issue direct priority and issue priority based on labels.

@wxiaoguang
Copy link
Contributor

I am reading these two PRs. Some thoughts:

Label-based priority

It doesn't introduce new concept for users, GitLab does so, and I agree about update issue with label's max weight and use label group to ensure single priority label for each issue.

However, I think there could be something difficult for end users. For example, if label group is used, could a user set two label groups with individual priorities? If yes, then the priority for the issue is still unclear; if no, Gitea need more checks for everything.

And it would cause some arguments, like https://gitlab.com/gitlab-org/gitlab/-/issues/14523 Issue priority should be determined by combination of label priorities instead of highest label priority . I do not think the behavior is clear for everyone.

PR #11669 used hard-coded priority levels, it may not satisfy real users. If it introduces customized priorities, it will be much more complex then.

Issue-based priority

It introduces a new concept on issue, while I think it's fine. Some issue tracker like Jira also does so, it seems more simple and easily to be understood.

If there is no Cons for Issue-based priority, I would prefer it personally.

wxiaoguang
wxiaoguang reviewed Sep 23, 2022
View reviewed changes
Copy link
Contributor

@wxiaoguang wxiaoguang left a comment

Choose a reason for hiding this comment

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

Is the PR almost done, or far from done?

routers/web/repo/issue_priority.go
Comment on lines +103 to +104
// UpdateIssueLabel change issue's labels
func UpdateIssuePriority(ctx *context.Context) {
Copy link
Contributor

Choose a reason for hiding this comment

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

label or priority?

Below code are for labels if I understand correctly

Copy link
Member Author

Choose a reason for hiding this comment

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

I haven't updated function comments

Copy link
Contributor

Choose a reason for hiding this comment

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

Not only the comment, but the code in UpdateIssuePriority are all for ClearLabels/AddLabel/RemoveLabel.

routers/web/repo/issue_priority.go
ctx.Data["PageIsLabels"] = true

if ctx.HasError() {
ctx.Flash.Error(ctx.Data["ErrorMsg"].(string))
Copy link
Contributor

Choose a reason for hiding this comment

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

HasError already has the logic ctx.Flash.ErrorMsg = ctx.Data["ErrorMsg"].(string)

Copy link
Member Author

Choose a reason for hiding this comment

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

This is copy paste from labels.

@techknowlogick
Copy link
Member Author

In the future a migration could be made to switch from specific priorities over to labels, so we aren't locked into this specific direction.

@techknowlogick
Copy link
Member Author

Closing per discussion in chat.

@techknowlogick techknowlogick deleted the issue-priority branch September 27, 2022 21:21
@lunny lunny removed this from the 1.18.0 milestone Dec 20, 2022
@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.
Reviewers

@wxiaoguang wxiaoguang wxiaoguang left review comments

Assignees
No one assigned
Labels
lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. type/feature Completely new functionality. Can only be merged if feature freeze is not active.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@techknowlogick @lafriks @lunny @wxiaoguang @GiteaBot