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

Added short and long form for month. #8

Closed
wants to merge 1 commit into from
Closed

Added short and long form for month. #8

wants to merge 1 commit into from

Conversation

heymartinadams
Copy link

Great package! Awesome that it’s so small. Not sure if you’d like to add short and long form for month, @lukeed. Increased size from 340B to to 366B.

@lukeed
Copy link
Owner

lukeed commented Jul 29, 2019

Hey, thanks~!

I would not – sorry 😞 I'll get around to adding in #7 in the near-ish future which will allow you to do whatever you want. It's been on my todo list... will try to fast track it this week.


By the way, it's generally a good idea to open an issue first to discuss new features so that you don't risk spending time on something that doesn't land. I appreciate the effort & time you do put into it, but it makes me feel guilty for the same reason. Also, I'd recommend not changing a project's code style. It hurts your chances at getting merged. Feature aside, I would have rejected/requested that you revert those changes – tinydate is purposefully written in ES5 to avoid a compilation step.

Thank you & sorry again~!
I'll get a move on #7 asap

Cheers

@lukeed lukeed closed this Jul 29, 2019
@heymartinadams
Copy link
Author

heymartinadams commented Jul 29, 2019

That totally makes sense, thanks, @lukeed. I’ll open an issue first, next time.

Also, thanks for explaining why you use ES5. And, yes, will keep code style as-is, next time I issue a PR. Not a problem, thanks for considering.

Ciao

@lukeed
Copy link
Owner

lukeed commented Jul 29, 2019

Cool, thank you for your understanding :)

Custom dictionaries are now available under v1.2.0~! I used your examples here as the illustrative example.

Thanks again 🙌

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.

2 participants