Skip to content

Conversation

@georgd
Copy link
Contributor

@georgd georgd commented Mar 10, 2022

No description provided.

});
});

// test("Test - Strict mode", function () {
Copy link
Owner

Choose a reason for hiding this comment

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

Please don't leave the commented out code like this.
If we don't have DE strict mode, let's remove it.

});
});

//test("Test - Nested time ago", function () {
Copy link
Owner

Choose a reason for hiding this comment

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

Same as previous comment about not leaving commented code.

If we are not ready to support the nested time ref, lets remove this and add it later.

}

const unit = TIME_UNIT_DICTIONARY[match[4].toLowerCase()];
let timeUnits = {};
Copy link
Owner

Choose a reason for hiding this comment

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

Nit: first, I'd rename num to value and I think let timeUnits = { unit: num }; is clearer to understand.

@wanasit
Copy link
Owner

wanasit commented Mar 28, 2022

Hey @georgd. Thanks for the improvement!
Could you please address or reply my comments?

@dpschen
Copy link

dpschen commented Mar 19, 2023

Hey @georgd, this is really useful! In case you currently don’t have time to continue on this would you be fine if I would pick up this PR?

@georgd
Copy link
Contributor Author

georgd commented Mar 19, 2023

Hi, indeed I sadly won't have time for this for another couple of months, as it seems. I'd be happy to hand this over to you 😀

@georgd
Copy link
Contributor Author

georgd commented Mar 19, 2023

Back then, I started refactoring the code to bring the DE part on par with EN but I am better with regex than with js so didn't get far in the little time I had. When I find the time, would you mind me proposing improvements?

@dpschen
Copy link

dpschen commented Mar 23, 2023

I'm not sure how much time I'll have. But probably it won't take a couple of months 😄

When I find the time, would you mind me proposing improvements?

Sure!

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