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

Subtract 1 from months when making JavaScript Dates for use in Google charts #474

Merged
merged 1 commit into from
Dec 14, 2018

Conversation

srowen
Copy link
Contributor

@srowen srowen commented Dec 13, 2018

Subtract 1 from months when making JavaScript Dates for use in Google charts. JS months are 0-indexed.

Description of changeset:

I believe this resolves #278
This seems to be the jinja2 way of subtracting from a variable's value:
http://jinja.pocoo.org/docs/2.10/templates/#math

Test Plan:

./run-tests.sh passes. I admit I'm not sure how to run the locally built copy of knowledge repo to check the change.

Auto-reviewers: @NiharikaRay @matthewwardrop @earthmancash @danfrankj

@AppVeyorBot
Copy link

@matthewwardrop
Copy link
Collaborator

@srowen Thanks for this patch as well! It looks good. Should the day also be zero-indexed?

@srowen
Copy link
Contributor Author

srowen commented Dec 13, 2018

Thanks again for the tip on running locally @matthewwardrop . Yes it seems to fix the display issue.
No only months are 0-indexed: https://www.w3schools.com/js/js_date_methods.asp
This mirrors Java FWIW.

@matthewwardrop
Copy link
Collaborator

@srowen This patch looks good also. I'll merge it in. Thanks for fixing these papercut issues.

Thanks also for being thorough and for the JS reference. I've done quite a bit of JS work myself in other projects, including work with dates, but always end up having to check the idiosyncrasies of each datetime library lest I make these off-by-one errors (like this one, though at least this one in particular was not of my hand ;).

@matthewwardrop matthewwardrop merged commit 6dc5b53 into airbnb:master Dec 14, 2018
@srowen
Copy link
Contributor Author

srowen commented Dec 14, 2018

Thanks! just little things indeed but help our internal usage at Databricks, where it's pretty helpful. Maybe I can add our org to INTHEWILD.md

@srowen srowen deleted the Issue278 branch December 14, 2018 15:27
@matthewwardrop
Copy link
Collaborator

@srowen Feel free to add your org to the INTHEWILD.md file :). Glad it is useful to you!

tanuj208 pushed a commit to ElucidataInc/knowledge-repo that referenced this pull request Jun 6, 2019
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.

Google charts - month indexing starts at 0
3 participants