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

phetioInstanceDocumentation is too verbose #63

Closed
zepumph opened this issue Oct 1, 2018 · 3 comments
Closed

phetioInstanceDocumentation is too verbose #63

zepumph opened this issue Oct 1, 2018 · 3 comments

Comments

@zepumph
Copy link
Member

zepumph commented Oct 1, 2018

For an option that is basically needed for every PhET-iO instrumented object, this is very long, from slack:

Chris Malley [13:44]
Question about the option `phetioInstanceDocumentation`…  Since it’s provided as an option to a constructor, the “Instance” bit seems redundant/unnecessary.  I’m asking because I’ve had to add this 45 times in graphing-quadratics, and I’ve been instructed that most things that get tandems should also get  `phetioInstanceDocumentation`.  Highly recommended to choose a less verbose name. (edited)

Michael Kauzmann [13:47]
I like that idea! `phetioInstanceDocumentation` --> `phetioDocumentation`. There is then more of a blur between that option and the 'documentation` property passed in as a static property to phetioInherit.

Chris Malley [13:47]
`phetioDoc` ?

Michael Kauzmann [13:48]
That will be exposed to the public API

Chris Malley [13:48]
even `phetioDocumentation` would be an improvement.


Michael Kauzmann [13:48]
I will make an issue.

I'm not opposed to phetioDoc if we can convince ourselves that it is clear enough, and that the space saving is worth the "code smell" (in the context to PhET's coding conventions: Names (types, variables, properties, functions,...) should be sufficiently descriptive and specific, and should avoid non-standard abbreviations. For example:).

It was in the slack, but it should be noted that whatever we choose will be exposed on the public api, and should be updated once and for all before stable FL to make it that much easier to maintain.

@samreid what do you think?

@samreid
Copy link
Member

samreid commented Oct 2, 2018

phetioDocumentation sounds nice to me. @zepumph would you like to proceed with the renaming?

@samreid samreid removed their assignment Oct 2, 2018
zepumph added a commit to phetsims/shred that referenced this issue Oct 4, 2018
zepumph added a commit to phetsims/joist that referenced this issue Oct 4, 2018
zepumph added a commit to phetsims/sun that referenced this issue Oct 4, 2018
@zepumph
Copy link
Member Author

zepumph commented Oct 4, 2018

@samreid please review. It was a pretty straight forward issue although it touched many files.

@samreid
Copy link
Member

samreid commented Oct 11, 2018

Looks great and tests in studio well, thanks, closing.

@samreid samreid closed this as completed Oct 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants