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

Skater tandem names don't match their associated Property names #358

Open
pixelzoom opened this issue Oct 27, 2016 · 9 comments
Open

Skater tandem names don't match their associated Property names #358

pixelzoom opened this issue Oct 27, 2016 · 9 comments

Comments

@pixelzoom
Copy link
Contributor

Noted while working on https://github.com/phetsims/phet-io/issues/661.

Skater has many tandem names that don't match their associated Property names (which is supposed to be the case, by convention.) This made it significantly more difficult to convert.

pixelzoom added a commit that referenced this issue Oct 27, 2016
@pixelzoom
Copy link
Contributor Author

All such Properties are marked with a TODO item that references this issue.

@samreid
Copy link
Member

samreid commented Nov 20, 2016

Marked for phet-io meeting to decide on the variable names.

@samreid samreid removed their assignment Nov 20, 2016
@samreid samreid self-assigned this Nov 21, 2016
@samreid
Copy link
Member

samreid commented Nov 21, 2016

We decided to use shorter but not cryptic names, like parametricPosition and parametricSpeed. @samreid should take care of this.

@samreid
Copy link
Member

samreid commented Dec 6, 2016

Fixed above and tested for about 5 minutes. This was an invasive change and possible that I've missed some situations. @ariel-phet I recommend that this be more thoroughly tested by the QA team before we publish another version out of master. It can be tested directly from phettest, or we can create a dev publication for testing. Reassigned to @ariel-phet for feedback.

@samreid samreid assigned ariel-phet and unassigned samreid Dec 6, 2016
@ariel-phet
Copy link

@samreid yes, please create a dev version, also create a test task with some context description (and perhaps some things that might be good to look out for). Please assign the test task to me at first.

@marlitas
Copy link
Contributor

This will be tested as part of the upcoming region and culture publication.

@marlitas marlitas self-assigned this Oct 31, 2023
@arouinfar
Copy link

This will be tested as part of the upcoming region and culture publication.

@marlitas are you sure? PhET-iO instrumentation of this sim is ancient (instance proxies) and isn't part of the Hydrogen set. If we redeploy ESPB from main as part of the localization work, I strongly recommend it is only phet-brand.

@marlitas
Copy link
Contributor

Ahhh I did not know this, thanks for the heads up @arouinfar. @kathy-phet this sounds like we will not include phet-io work for region and culture publications then?

@marlitas
Copy link
Contributor

PhET-iO brand is not included in the upcoming region and culture publication. Marking as deferred.

@marlitas marlitas removed their assignment Oct 31, 2023
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

5 participants