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

Add format function from the README #5

Closed
wants to merge 4 commits into from
Closed

Add format function from the README #5

wants to merge 4 commits into from

Conversation

HoldYourWaffle
Copy link
Contributor

I based this PR of #3 so I could immediately update the type definitions

I added the formatting function from the README to the library with some additional options for black/whitelisting certain properties and changing the 'glue' used.
Since this is a breaking change (different imports) I also updated the major version number for your convenience.

If you decide to merge this I could also update the documentation (and include a short 'migration' section).

@nikitaeverywhere
Copy link
Owner

nikitaeverywhere commented May 10, 2019

Hello! Thank you so much for your work, I appreciate that!

I've reviewed your pull requests #3 & #4, they're awesome, thanks! I'll merge them in a short while.

However, this PR looks a bit strange to me. The point of those lines of code I put in the readme regarding formatting was just to mention the quick example of how to use the library. The formatting is not something that this library deserves, I have a strong opinion about that. The formatting is a thing where people usually end up using a whole another library (for example), for a reason: you cannot write a function which covers a will of all the library users. Check how much readable it turns to be while using the library:

import getDateDiff from "datetime-difference";
import format from "string-format";

const dateDiff = getDateDiff(new Date(), new Date()); // ...
console.log(
    format("{days} days left", dateDiff)
);

No glue, no custom functions, the freedom in choosing how to format a thing. (this PR also breaks the compatibility with node v4)

So I would strongly advise you to follow this path, keeping the library as minimalistic as possible. What makes sense though, is to make a PR adding the example above to the readme file :) Done

Thanks!

@HoldYourWaffle HoldYourWaffle deleted the format-func branch May 10, 2019 21:40
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

2 participants