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

Issue time estimate, meaningful time tracking #23113

Merged
merged 117 commits into from
Dec 5, 2024
Merged

Issue time estimate, meaningful time tracking #23113

merged 117 commits into from
Dec 5, 2024

Conversation

stuzer05
Copy link
Contributor

@stuzer05 stuzer05 commented Feb 24, 2023 •
edited
Loading

This pr does:

  1. Adds functionality to set estimated time for issues similar to Jira (in "1w 3d 15h 30m" format)
  2. Now it's possible to view exact tracking time both logged, total and per-user (estimated time is not useful for tracking)
  3. Replaces Stopwatch "Add time" form from Hours and MInutes to Jira-like input in "1w 3d 15h 30m" format
  4. Adds ability to localize all time tracking issue comments in the future
  5. Removes meaningless seconds from tracking display information
  6. Makes time logging comments the same style as other comments (no new line and icon)

image
image

Why?

Time estimation is essential for some teams to know the importance of issues, the amount of effort an issue would take. And for business to know if it's profitable to spend money on developers time to implement an issue

More info on https://support.atlassian.com/jira-software-cloud/docs/estimate-an-issue/#Concepts-about-estimation

Closes #23112

@stuzer05 stuzer05 mentioned this pull request Feb 24, 2023
Closed
@lunny lunny added the type/feature Completely new functionality. Can only be merged if feature freeze is not active. label Feb 24, 2023
@lunny lunny added this to the 1.20.0 milestone Feb 24, 2023
yardenshoham
yardenshoham suggested changes Feb 24, 2023
View reviewed changes
Copy link
Member

@yardenshoham yardenshoham left a comment

Choose a reason for hiding this comment

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

Please make sure you've read https://github.com/go-gitea/gitea/blob/main/CONTRIBUTING.md

models/issues/comment.go Outdated Show resolved Hide resolved
options/locale/locale_ru-RU.ini Outdated Show resolved Hide resolved
options/locale/locale_uk-UA.ini Outdated Show resolved Hide resolved
routers/web/repo/issue.go Outdated Show resolved Hide resolved
services/issue/issue.go Outdated Show resolved Hide resolved
services/issue/issue.go Outdated Show resolved Hide resolved
services/issue/issue.go Outdated Show resolved Hide resolved
@GiteaBot GiteaBot added lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Feb 24, 2023
stuzer05 and others added 5 commits February 24, 2023 13:35
@stuzer05 stuzer05 changed the title #23112 Ability to add issue plan time #23112 Ability to add issue time estimate Feb 24, 2023
@yardenshoham
Copy link
Member

I suggest you remove the issue number from the title and add Closes #23112 to the PR description. Also mark this PR as draft while you're working on it.

@stuzer05
Copy link
Contributor Author

stuzer05 commented Dec 3, 2024

I have some ideas to improve the UI now (after refactoring the issue sidebar)

Will work on it to address all the concerns above:

  1. reduce UI complexity (lunny: Maybe we need a better sidebar design, there are too items for time.)
  2. simplify the time unit: (lafriks: My proposal would be to display estimate only in hours&minutes and later on implement other math behind this using org/global settings etc)
  3. add some tests (6543: adding tests for this function would be awesome!)

Then I think we can get it in 1.23

Thank you for continuing on this!

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Dec 3, 2024 •
edited
Loading

@wxiaoguang why still days? It still will be wrongly interpreted this way (1 astronomical day != 1 work day). Apart from that looks good code wise

Oops, I made a mistake. Please take a look at 4bac5a0 , and I added enough comments there.

// When tracking working time, only hour/minute/second units are accurate and could be used.
// For other units like "day", it depends on "how many working hours in a day": 6 or 7 or 8?
// So at the moment, we only support hour/minute/second units.
// In the future, it could be some configurable options to help users
// to convert the working time to different units.

@wxiaoguang wxiaoguang force-pushed the add-issue-planned-time branch from 9377c1d to 4bac5a0 Compare December 3, 2024 07:23
@wxiaoguang wxiaoguang removed their assignment Dec 3, 2024
lunny
lunny approved these changes Dec 3, 2024
View reviewed changes
Copy link
Member

@lunny lunny left a comment

Choose a reason for hiding this comment

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

Unit test failure is related, otherwise LGTM

@pull-request-size pull-request-size bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Dec 4, 2024
@wxiaoguang wxiaoguang force-pushed the add-issue-planned-time branch from 00ca5e3 to a15d976 Compare December 4, 2024 00:14
@wxiaoguang
Copy link
Contributor

Unit test failure is related, otherwise LGTM

CI passes, last call for reviews (especially the reviewers who have concerns and change-requests)

@lunny
Copy link
Member

lunny commented Dec 4, 2024

@lafriks needs your review

lafriks
lafriks approved these changes Dec 4, 2024
View reviewed changes
Copy link
Member

@lafriks lafriks left a comment

Choose a reason for hiding this comment

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

Looks good, thanks 😉

I don't see much point in seconds either as I don't see how any estimate could be less than 5 minutes or so exact but it's not blocking

@lunny lunny requested review from yardenshoham and removed request for yardenshoham and 6543 December 4, 2024 06:21
@lunny
Copy link
Member

lunny commented Dec 5, 2024

I will merge this one manually if there is no response from @yardenshoham . I couldn't find a real unresolved block message.

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/blocked A maintainer has reservations with the PR and thus it cannot be merged labels Dec 5, 2024
@wxiaoguang wxiaoguang enabled auto-merge (squash) December 5, 2024 12:44
@wxiaoguang wxiaoguang merged commit 936665b into go-gitea:main Dec 5, 2024
26 checks passed
zjjhot added a commit to zjjhot/gitea that referenced this pull request Dec 6, 2024
@zjjhot
Merge remote-tracking branch 'giteaofficial/main'
5b3d5b9 
* giteaofficial/main:
  Bump relative-time-element to v4.4.4 (go-gitea#32730)
  Update dependencies, tweak eslint (go-gitea#32719)
  Issue time estimate, meaningful time tracking (go-gitea#23113)
@stuzer05 stuzer05 deleted the add-issue-planned-time branch December 7, 2024 15:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@delvh delvh delvh left review comments

@6543 6543 6543 left review comments

@silverwind silverwind silverwind left review comments

@yardenshoham yardenshoham yardenshoham approved these changes

@wxiaoguang wxiaoguang wxiaoguang approved these changes

@lafriks lafriks lafriks approved these changes

@lunny lunny lunny approved these changes

Assignees
No one assigned
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. modifies/frontend modifies/go Pull requests that update Go code modifies/migrations modifies/templates This PR modifies the template files modifies/translation outdated/theme/timetracker size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. type/feature Completely new functionality. Can only be merged if feature freeze is not active.
Projects
None yet
Milestone
1.23.0
Development

Successfully merging this pull request may close these issues.

Issue time estimate, meaningful time tracking