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

Tandem name should match associated variable/field name #55

Closed
pixelzoom opened this issue Jun 28, 2022 · 2 comments
Closed

Tandem name should match associated variable/field name #55

pixelzoom opened this issue Jun 28, 2022 · 2 comments

Comments

@pixelzoom
Copy link
Contributor

pixelzoom commented Jun 28, 2022

For code review #41 ...

By PhET-iO convention, tandem names are typically supposed to match their associated variable/field name (with exceptions made for unusual cases). There are at least 5 violations of that in this sim, and none of them qualifies as "unusual".

For example, in MeanShareAndBalanceScreenView.ts:

this.syncDataButton = new RectangularPushButton( {
...
      tandem: options.tandem.createTandem( 'syncRepresentationsButton' ),

I added comment "//REVIEW tandem name does not match" to all of the cases that I identified. (Not necessarily a complete list.)

@pixelzoom pixelzoom changed the title Tandem name should match associated variable/property name Tandem name should match associated variable/field name Jun 29, 2022
@marlitas
Copy link
Contributor

Did a search and fixed all the instances I was able to find. Back to @pixelzoom for re-review.

@pixelzoom
Copy link
Contributor Author

👍🏻 closing.

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