Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Reimplement duration rounding#301

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
leduyquang753 wants to merge3 commits intogithub:main
base:main
Choose a base branch
Loading
fromleduyquang753:rounding

Conversation

leduyquang753
Copy link
Contributor

@leduyquang753leduyquang753 commentedDec 31, 2024•
edited
Loading

(This is part of a set of multiple pull requests looking to overhaul the calculation functions.)

This pull request reimplements theroundToSingleUnit function with more robust logic. The original date is now computed through theapplyDuration function (a new implementation is in#298).

As this is an isolated pull request, tests will appear to fail until changes from the other pull requests of the set are integrated.

@leduyquang753leduyquang753 requested a review froma team as acode ownerDecember 31, 2024 10:30
@leduyquang753
Copy link
ContributorAuthor

I have moved the implementation ofrelativeTime to apply toroundToSingleUnit as per@keithamus's suggestion in#300.

Copy link
Contributor

@keithamuskeithamus left a comment

Choose a reason for hiding this comment

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

I think this is looking good 👍. There are some failing tests which need to be addressed please, but overall I like the shape of this.

@leduyquang753
Copy link
ContributorAuthor

leduyquang753 commentedJan 13, 2025•
edited
Loading

As this is an isolated pull request, tests will appear to fail until changes from the other pull requests of the set are integrated.

This is expected and I have mentioned in my new pull request description: "As this is an isolated pull request, tests will appear to fail until changes from the other pull requests of the set are integrated."

Particularly, the tests are based on the usage of the new implementations ofapplyDuration andelapsedTime from the other pull requests.

@keithamus
Copy link
Contributor

This is expected and I have mentioned in my new pull request description

That's fine, more meant as a note for colleagues. I'd like us to merge in an order which maintains the health of the main branch, if we can (despite the current failing test in main).

the other pull requests

Which other PRs do you anticipate will reduce the test failures on this one?

@leduyquang753
Copy link
ContributorAuthor

Which other PRs do you anticipate will reduce the test failures on this one?

My new implementation ofapplyDuration in#298 and ofelapsedTime in#299.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@keithamuskeithamuskeithamus approved these changes

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

2 participants
@leduyquang753@keithamus
[8]ページ先頭
©2009-2025 Movatter.jp