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.

Sign up for GitHub

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

Jump to bottom

format utils cleanup #71

Merged
merged 15 commits into from
May 3, 2017
Merged

format utils cleanup #71

merged 15 commits into from
May 3, 2017

Conversation

jebeck
Copy link
Contributor

@jebeck jebeck commented May 2, 2017 •
edited
Loading

Again as planned @krystophv

The last commit isn't the greatest because I re-alphabetized (for ease of comparing across source and test files to make sure everything's covered), but otherwise commit-by-commit would be a good strategy for review I'd imagine.

There is a blip PR that will depend on this since I standardized on passing through entire bgPrefs to the TrendsContainer and Trends tooltip components.

jebeck added 5 commits May 2, 2017 10:25
@jebeck
add guidelines parallel to those added to datetime utils
919f324
@jebeck
displayDecimal -> formatDecimalNumber, tweaks to JSDoc
2bf2441
@jebeck
displayBgValue -> formatBgValue, standardize 2nd param to entire bgPr…
be46f7c 
…efs object
@jebeck
patientFullName -> getPatientFullName and move from format to misc ut…
7e5c073 
…ils b/c doesn't do any formatting
@jebeck
a few changes
a197e86 
- birthday -> formatBirthdate and move to datetime utils
- diagnosisDate -> formatDiagnosisDate and move to datetime utils
- alphabetize format and datetime utils & tests
- add a note in guidelines re: alphabetizing utils & tests
@jebeck jebeck requested a review from krystophv May 2, 2017 19:24
@jebeck jebeck mentioned this pull request May 2, 2017
Merged
@jebeck
Copy link
Contributor Author

jebeck commented May 2, 2017

[blip] Print View - DAILY

@jebeck jebeck mentioned this pull request May 2, 2017
Merged
krystophv
krystophv approved these changes May 3, 2017
View reviewed changes
Copy link
Member

@krystophv krystophv left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@jebeck jebeck merged commit d476010 into master May 3, 2017
@jebeck jebeck deleted the jebeck/format-utils-cleanup branch May 3, 2017 19:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@krystophv krystophv krystophv approved these changes

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
@jebeck @krystophv