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

code review #78

Closed
samreid opened this issue Nov 13, 2016 · 1 comment
Closed

code review #78

samreid opened this issue Nov 13, 2016 · 1 comment
Assignees
@samreid

Comments

@samreid
Copy link
Member

samreid commented Nov 13, 2016

Step 1 of PhET-iO instrumentation is a code review. @aadish did a code review in #48 starting in Jan 2016, which is mostly complete. So I'll do a cursory look, mostly through the code style, organization, etc as it pertains to PhET-iO instrumentation.
@samreid samreid self-assigned this Nov 13, 2016
samreid added a commit that referenced this issue Nov 13, 2016
@samreid
Reformatted and updated some comments, see #78
6689cf5
samreid added a commit that referenced this issue Nov 13, 2016
@samreid
Renamed rulerProperty=>rulerPositionProperty and changed type to Vect…
5f23ab9 
…or2, see #78
samreid added a commit that referenced this issue Nov 13, 2016
@samreid
Converted forceProperty to derived property, see #78
e9fc366
samreid added a commit that referenced this issue Nov 13, 2016
@samreid
Fixed var declaration site, see #78
8d916a2
samreid added a commit that referenced this issue Nov 13, 2016
@samreid
Cleaned up whitespace and comments and removed modelViewTransform ref…
2fc48a2 
…erence, see #78
samreid added a commit that referenced this issue Nov 13, 2016
@samreid
Simplified unnecessary methods, see #78
0e66d7a
This was referenced Nov 13, 2016
Closed
Closed
samreid added a commit that referenced this issue Nov 13, 2016
@samreid
Simplified setting ruler translation, see #78
619dd2c
samreid added a commit that referenced this issue Nov 13, 2016
@samreid
Renamed var, see #78
e195c88
samreid added a commit that referenced this issue Nov 13, 2016
@samreid
Factored out CONTROL_SCALE, see #78
a5be54c
samreid added a commit that referenced this issue Nov 13, 2016
@samreid
Simplified constantRadiusProperty.link, see #78
7d77e6a
samreid added a commit that referenced this issue Nov 13, 2016
@samreid
Simplified Mass control contents, see #78
ff611ac
@samreid samreid mentioned this issue Nov 13, 2016
Closed
samreid added a commit that referenced this issue Nov 14, 2016
@samreid
Factored out constant, see #78
f11f1bd
samreid added a commit that referenced this issue Nov 14, 2016
@samreid
Minor cleanup, see #78
f72362d
samreid added a commit that referenced this issue Nov 14, 2016
@samreid
Updated docs, see #78
68ee0b6
@samreid
Copy link
Member Author

samreid commented Nov 20, 2016

Seems pretty good, and @jbphet and I addressed a few other issues as we instrumented the simulation.

@samreid samreid closed this as completed Nov 20, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@samreid samreid

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@samreid