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

Reword millisecond docs to give example of usage #376

Merged
merged 1 commit into from Aug 29, 2015

Conversation

Projects
None yet
2 participants
@eeue56
Contributor

eeue56 commented Aug 28, 2015

As per #373, makes millisecond docs simpler to understand

@jvoigtlaender

This comment has been minimized.

Show comment
Hide comment
@jvoigtlaender

jvoigtlaender Aug 28, 2015

Contributor

I don't really like the talk of "variable" here (in the new documentation comment). Does the core documentation use language like "variable" elsewhere?

Generally, wouldn't a much easier fix to the problematic aspect of the documentation you point out be to simply replace (500 * milliseconds) by (500 * millisecond)? Why is the rest of the rewording necessary from your perspective?

Contributor

jvoigtlaender commented Aug 28, 2015

I don't really like the talk of "variable" here (in the new documentation comment). Does the core documentation use language like "variable" elsewhere?

Generally, wouldn't a much easier fix to the problematic aspect of the documentation you point out be to simply replace (500 * milliseconds) by (500 * millisecond)? Why is the rest of the rewording necessary from your perspective?

@eeue56

This comment has been minimized.

Show comment
Hide comment
@eeue56

eeue56 Aug 28, 2015

Contributor

I've removed the word variable now - I agree it didn't feel right there.

That would be an easier fix, but a less clear fix I feel. I wanted an explicit code example that people could use right away, rather an example just in the brackets, which doesn't seem as obvious that it's meant to be code. Adding halfSecond = 500 * millisecond makes it more explicit and obvious than (500 * millisecond) in the middle of a sentence, basically.

Contributor

eeue56 commented Aug 28, 2015

I've removed the word variable now - I agree it didn't feel right there.

That would be an easier fix, but a less clear fix I feel. I wanted an explicit code example that people could use right away, rather an example just in the brackets, which doesn't seem as obvious that it's meant to be code. Adding halfSecond = 500 * millisecond makes it more explicit and obvious than (500 * millisecond) in the middle of a sentence, basically.

@eeue56

This comment has been minimized.

Show comment
Hide comment
@eeue56

eeue56 Aug 28, 2015

Contributor

Though I could just change it to remove the s if you think that's clear enough.

Contributor

eeue56 commented Aug 28, 2015

Though I could just change it to remove the s if you think that's clear enough.

@jvoigtlaender

This comment has been minimized.

Show comment
Hide comment
@jvoigtlaender

jvoigtlaender Aug 29, 2015

Contributor

I actually think just removing the plural s would be enough.

Contributor

jvoigtlaender commented Aug 29, 2015

I actually think just removing the plural s would be enough.

@jvoigtlaender

This comment has been minimized.

Show comment
Hide comment
@jvoigtlaender

jvoigtlaender Aug 29, 2015

Contributor

That it's code, should be clear from type-setting on the docs page.

Contributor

jvoigtlaender commented Aug 29, 2015

That it's code, should be clear from type-setting on the docs page.

@eeue56 eeue56 closed this Aug 29, 2015

@eeue56 eeue56 reopened this Aug 29, 2015

@eeue56

This comment has been minimized.

Show comment
Hide comment
@eeue56

eeue56 Aug 29, 2015

Contributor

Okay - changed it so it's now just the 's' removal, and got rid of the previous commits I made too.

Contributor

eeue56 commented Aug 29, 2015

Okay - changed it so it's now just the 's' removal, and got rid of the previous commits I made too.

jvoigtlaender added a commit that referenced this pull request Aug 29, 2015

Merge pull request #376 from eeue56/patch-3
Reword millisecond docs to use correct function name.

@jvoigtlaender jvoigtlaender merged commit 8918d7e into elm:master Aug 29, 2015

1 check failed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment