Skip to content

Commit

Permalink
promote REVIEW comments to issue, #279, #269
Browse files Browse the repository at this point in the history
Signed-off-by: Chris Malley <[email protected]>
  • Loading branch information
pixelzoom committed Dec 19, 2018
1 parent 36d5f91 commit 1f9668a
Show file tree
Hide file tree
Showing 2 changed files with 0 additions and 13 deletions.
7 changes: 0 additions & 7 deletions js/common/model/Scene.js
Original file line number Diff line number Diff line change
Expand Up @@ -122,9 +122,6 @@ define( require => {
units: this.timeUnits
} );

//REVIEW add value validation?
//REVIEW* the model supports any value, though going outside the bounds of the lattice would probably be a logic
//REVIEW* error. The view slider has its own constraint. Which value should I use here?
// @public distance between the sources in the units of the scene, or 0 if there is only one
// source initialized to match the initial slit separation,
// see https://github.com/phetsims/wave-interference/issues/87
Expand Down Expand Up @@ -154,15 +151,11 @@ define( require => {
barrierLocation => Math.round( barrierLocation.x )
);

//REVIEW add value validation? range?
//REVIEW* See comments regarding this.sourceSeparationProperty. How should I proceed?
// @public {NumberProperty} - width of the slit(s) opening in the units for this scene
this.slitWidthProperty = new NumberProperty( config.initialSlitWidth, {
units: this.positionUnits
} );

//REVIEW add value validation?
//REVIEW* See comments regarding this.sourceSeparationProperty. How should I proceed?
// @public distance between the center of the slits, in the units for this scene
this.slitSeparationProperty = new NumberProperty( config.initialSlitSeparation, {
units: this.positionUnits
Expand Down
6 changes: 0 additions & 6 deletions js/common/model/WaterScene.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,20 +25,14 @@ define( require => {
constructor( config ) {
super( config );

//REVIEW add value validation? range: this.frequencyProperty.range ?
//REVIEW* Please see question in Scene.sourceSeparationProperty REVIEW* comment
// @public - In the water Scene, the user specifies the desired frequency and amplitude, and that
// gets propagated to the lattice via the water drops
this.desiredFrequencyProperty = new NumberProperty( this.frequencyProperty.initialValue );

//REVIEW add value validation?
//REVIEW* Please see question in Scene.sourceSeparationProperty REVIEW* comment
// @public - In the water Scene, the user specifies the desired source separation. This is the position of
// the faucets. The sourceSeparationProperty indicates the sources of oscillation once the water has struck.
this.desiredSourceSeparationProperty = new NumberProperty( this.sourceSeparationProperty.value );

//REVIEW add value validation?
//REVIEW* Please see question in Scene.sourceSeparationProperty REVIEW* comment
// @public - the amplitude the user has selected
this.desiredAmplitudeProperty = new NumberProperty( config.initialAmplitude );

Expand Down

0 comments on commit 1f9668a

Please sign in to comment.