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

Gitlab Error in merge request with estimate or spent time #548

Merged
merged 4 commits into from
Dec 16, 2022

Conversation

oscarcv
Copy link

@oscarcv oscarcv commented Nov 27, 2022

The humanTimeEstimate and humanTimeSpent fields are parsed as Int and the api returns them as String.

An example of this format would be 1mo 2w 3d 4h 5m (Gitlab - Time Tracker Units)

An example from the official gitlab documentation:

{
  "human_time_estimate": "3h 30m",
  "human_total_time_spent": null,
  "time_estimate": 12600,
  "total_time_spent": 0
}

Thank you

@oscarcv oscarcv changed the title Gitlab Error in merge request with estimate or spend time Gitlab Error in merge request with estimate or spent time Nov 27, 2022
Copy link
Member

@f-meloni f-meloni left a comment

Choose a reason for hiding this comment

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

Good catch!
Can you please also update the changelog?
As an addition having a unit test for this would be great, but I'm fine also without it

@oscarcv
Copy link
Author

oscarcv commented Dec 13, 2022

Good catch! Can you please also update the changelog? As an addition having a unit test for this would be great, but I'm fine also without it

@f-meloni

I have modified the tests to validate the type change and check the parse and I have also added the entry in the Changelog.

Thanks

@417-72KI
Copy link
Member

417-72KI commented Dec 14, 2022

@oscarcv
Can you please merge or rebase origin? 🙇
An linux runner on GitHub Actions has been updated and caused CI failure, resolved on #550.

@oscarcv
Copy link
Author

oscarcv commented Dec 14, 2022

@oscarcv Can you please merge or rebase origin? 🙇 An linux runner on GitHub Actions has been updated and caused CI failure, resolved on #550.

@417-72KI Merged from origin

@f-meloni f-meloni merged commit 086017c into danger:master Dec 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants