Skip to content

Fix reltime to seconds parsing #2969

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 2 commits into from
Mar 17, 2025
Merged

Fix reltime to seconds parsing #2969

merged 2 commits into from
Mar 17, 2025

Conversation

Kevinjil
Copy link
Contributor

@Kevinjil Kevinjil commented Mar 17, 2025

Extracted the refactor and bug fix of #2946 to a separate PR. The change in the API spec is not included in this change: I will rebase the temporary fix on main after merging this.

In the original code, the modifier was only applied to the hours of the reltime string. Note that test testRelTimeToSecondsWithNegative fails without the fix commit:

Failed asserting that -3477.0 matches expected -3723.

No behavioral changes intended: the negative modifier bug is explicitly
included in this refactoring.
@nickygerritsen
Copy link
Member

Does this still work with the non reltime format which we still will have in the current ICPC season?

@Kevinjil
Copy link
Contributor Author

Kevinjil commented Mar 17, 2025

Does this still work with the non reltime format which we still will have in the current ICPC season?

Yes: the spec change is not in this PR, only the refactoring and the bugfix. I will update my branch fix/penaltytime with the spec change ("ugly" fix) such that we can cherry-pick it for contests if we need it for as long as we don't implement the new spec properly.

@Kevinjil Kevinjil marked this pull request as draft March 17, 2025 19:24
@Kevinjil Kevinjil marked this pull request as ready for review March 17, 2025 19:25
@Kevinjil Kevinjil added this pull request to the merge queue Mar 17, 2025
Merged via the queue into main with commit 10b1c66 Mar 17, 2025
42 checks passed
@Kevinjil Kevinjil deleted the fix/reltime-parsing branch March 17, 2025 20:03
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.

3 participants