#269 review and remove all REVIEW threads

Signed-off-by: Chris Malley <[email protected]>

js/common/WaveInterferenceUtils.js

@@ -60,9 +60,6 @@ define( require => {
      */
     static getWaterSideY( waveAreaBounds, waveValue ) {
 
-      //REVIEW explain the magic numbers here
-      //REVIEW That addresses the 5, but not the -80.
-      //REVIEW* Done, please review.
       // Typical values for the propagating wave can be between -5 and 5 (though values can exceed this range very close
       // to the oscillating cell.  We choose to map a value of 0 to the center of the wave area, and the max (5) to the
       // desired distance amplitude.  A wave value of 0 appears in the center of the wave area. A value of 5 appears 80

js/common/model/LightScene.js

@@ -21,10 +21,6 @@ define( require => {
     constructor( config ) {
       super( config );
 
-      //REVIEW (read-only) ?
-      //REVIEW* The reference is read only, but clients can call mutators on it.  So how should it be marked?
-      //REVIEW With the exception of Property, I believe the PhET convention is that 'read-only' applies to assignment, not mutation. I added doc.
-      //REVIEW* Looks good, can this thread be removed?
       // @public (read-only) reads out the intensity on the right hand side of the lattice
       // While this is annotated as 'read-only' for assignment, it can be mutated by clients.
       this.intensitySample = new IntensitySample( this.lattice );

js/common/model/Scene.js

@@ -529,10 +529,6 @@ define( require => {
       this.phase = -this.timeProperty.value * angularFrequency;
     }
 
-    //REVIEW should the PhET convention for ES5 getters be used here? That is, define and call getWavelength.
-    //REVIEW* I don't see the value in having both.  OK if I just go with getWavelength?
-    //REVIEW Sure. I'm also OK with ES5 getter only, just pointing out that this is contrary to recent dev discussion.
-    //REVIEW* I went with getWavelength, can this be closed?
     /**
      * Returns the wavelength in the units of the scene
      * @returns {number}

js/common/model/SoundParticle.js

@@ -1,11 +1,5 @@
 // Copyright 2018, University of Colorado Boulder
 
-//REVIEW I don't understand the "When selected" bit.
-//REVIEW* I meant to indicate that the sound particles are only updated and displayed when the user
-//REVIEW* has selected the "sound" scene and one of the two options that display particles.
-//REVIEW* Can you recommend the best way to indicate this?
-//REVIEW: I modified the doc, please review.
-//REVIEW* I also added the word "wave", can this be closed?
 /**
  * When the sound wave generator is selected, shows discrete, moving particles for the sound scene.
  *

js/common/view/ToolboxPanel.js

@@ -63,10 +63,8 @@ define( require => {
       } );
 
       // Make sure the probes have enough breathing room so they don't get shoved into the WaveMeterNode icon.  Anything
-      // above 60 seems to work equally well, closer than that causes the probes to overlap eachh other or the meter
+      // above 60 seems to work equally well, closer than that causes the probes to overlap each other or the meter
       // body. The true translation is set when dragged out of the toolbox.
-      //REVIEW magic number?
-      //REVIEW* Please review newly added docs
       waveMeterNode.backgroundNode.translate( 60, 0 );
 
       // The draggable icon, which has an overlay to make the buttons draggable instead of pressable

js/common/view/WaveAreaNode.js

@@ -15,8 +15,6 @@ define( require => {
 
   class WaveAreaNode extends Rectangle {
 
-    //REVIEW model parameter is not used
-    //REVIEW* Removed
     /**
      * @param {Object} [options]
      */

js/common/view/WaveInterferenceControlPanel.js

@@ -170,8 +170,6 @@ define( require => {
       intensityCheckbox.mouseArea = intensityCheckbox.localBounds.dilated( 2 ).withX( separator.right );
       intensityCheckbox.touchArea = intensityCheckbox.mouseArea;
 
-      //REVIEW worthy of factoring out into its own class file, SceneRadioButtonGroup?
-      //REVIEW* I added SceneRadioButtonGroup.js, please review
       const sceneRadioButtonGroup = new SceneRadioButtonGroup(
         model.waterScene,
         model.soundScene,

js/common/view/WaveInterferenceText.js

@@ -23,10 +23,6 @@ define( require => {
       super( string, _.extend( {
         font: WaveInterferenceConstants.DEFAULT_FONT,
 
-        //REVIEW This seems like a sketchy thing to set for all Text in the sim. Why this value? Why handle it this way?
-        //REVIEW This addresses numerous cases where `maxWidth: 120` would have to otherwise be set in client code.
-        //REVIEW I added your reply as a comment here.
-        //REVIEW* Looks great, can this be closed?
         // This addresses numerous cases where `maxWidth: 120` would have to otherwise be set in client code.
         maxWidth: 120
       }, options ) );

js/slits/view/TheoryInterferenceOverlay.js

@@ -19,9 +19,7 @@ define( require => {
   // constants
   const LENGTH = 500;
   const MAXIMUM_COLOR = 'yellow';
-  const MINIMUM_COLOR = 'red'; //REVIEW should this be PhetColorScheme.RED_COLOR_BLIND?
-                               //REVIEW* Tracked in https://github.com/phetsims/wave-interference/issues/291
-                               //REVIEW* Can these comments be removed?
+  const MINIMUM_COLOR = 'red';
   const LINE_WIDTH = 1;
 
   class TheoryInterferenceOverlay extends Node {
@@ -34,9 +32,6 @@ define( require => {
     constructor( model, viewBounds, options ) {
       super( options );
 
-      //REVIEW why not pass options to super in previous statement?
-      //REVIEW* Done, please review
-
       const updateLines = () => {
         this.removeAllChildren();
         const barrierType = model.sceneProperty.value.barrierTypeProperty.get();

js/waves/model/WavesModel.js

@@ -56,8 +56,6 @@ define( require => {
   // This simulation uses EventTimer, which provides exactly the same model behavior on very slow and very fast
   // platforms.  Here we define the frequency of events in Hz, which has been tuned so that iPad2 has enough time to run
   // model computations.
-  //REVIEW say more about this value, effect of increasing/decreasing
-  //REVIEW* I added comments, please review.
   const EVENT_RATE = 20;
   const toFemto = WaveInterferenceUtils.toFemto;
 
@@ -86,12 +84,8 @@ define( require => {
         waveSpatialType: WaveSpatialType.POINT
       }, options );
 
-      //REVIEW assert && assert( WaveInterferenceConstants.AMPLITUDE_RANGE.contains( options.initialAmplitude ),
-      //REVIEW   'initialAmplitude is out of range: ' + options.initialAmplitude );
-      //REVIEW* Looks good, thanks!  Can this comment thread be removed?
       assert && assert( WaveInterferenceConstants.AMPLITUDE_RANGE.contains( options.initialAmplitude ),
-        'initialAmplitude is out of range: ' + options.initialAmplitude
-      );
+        'initialAmplitude is out of range: ' + options.initialAmplitude );
 
       assert && assert(
         options.numberOfSources === 1 || options.numberOfSources === 2,
@@ -221,8 +215,6 @@ define( require => {
         }
       };
 
-      //REVIEW might be nice to say a few words about this
-      //REVIEW* Done, please review.
       // @private - In order to have exactly the same model behavior on very fast and very slow platforms, we use
       // EventTimer, which updates the model at regular intervals, and we can interpolate between states for additional
       // fidelity.
@@ -267,8 +259,6 @@ define( require => {
       // @public
       this.isWaveMeterInPlayAreaProperty = new BooleanProperty( false );
 
-      //REVIEW since this applies to rotationAmountProperty, recommended to rename to rotationAmountRange
-      //REVIEW* Done, please review
       const rotationAmountRange = new Range( 0, 1 );
 
       // @public - Linear interpolation between ViewpointEnum.TOP (0) and ViewpointEnum.SIDE (1).  This linear