REVIEW notes, see phetsims/my-solar-system#88

js/view/CueingArrowsNode.ts

@@ -50,13 +50,17 @@ export default class CueingArrowsNode extends Path {
     super( createArrowsShape( options.bodyRadius, options.length ), options );
 
     this.length = options.length;
+
+    //REVIEW: Why is this being set? Shouldn't the rotation option (that is currently passed to the super) be sufficient?
+    //REVIEW: I believe this can be removed
     this.rotation = options.rotation;
 
     const updateTouchArea = () => {
       this.touchArea = this.localBounds.dilated( 5 );
       this.mouseArea = this.localBounds.dilated( 3 );
     };
     //REVIEW: This depends on the localBounds, so it should link to the localBoundsProperty instead
+    //REVIEW: THEN, in the link, you can use localBounds as a parameter instead of having to grab `this.localBounds`
     this.boundsProperty.link( updateTouchArea );
 
     //REVIEW: Don't need/want a disposal of the updateTouchArea, since they have the same lifetime

js/view/SolarSystemCommonTextNumberDisplay.ts

@@ -16,18 +16,25 @@ import SolarSystemCommonStrings from '../../../solar-system-common/js/SolarSyste
 import StringUtils from '../../../phetcommon/js/util/StringUtils.js';
 import solarSystemCommon from '../solarSystemCommon.js';
 
+//REVIEW: Single usage, prefer inlining (then don't have to define the type, AND won't need the extra empty {} object
+//REVIEW: allocation that is in the optionize3 call below). Can just use optionize
 const STRING_PATTERN_OPTIONS: NumberDisplayOptions = {
   backgroundFill: null,
   backgroundStroke: null,
   textOptions: SolarSystemCommonConstants.TEXT_OPTIONS,
   decimalPlaces: 1,
   useRichText: true,
+
+  //REVIEW: layoutOptions like this should be provided at the client usage. It's not great to hardcode client uses
+  //REVIEW: in the type itself. Let's move this to the client!
   layoutOptions: {
     align: 'left'
   }
 };
 
 export default class SolarSystemCommonTextNumberDisplay extends NumberDisplay {
+  //REVIEW: Ummm... I don't see this constructor ever getting used. Can we remove it, and not extend NumberDisplay?
+  //REVIEW: Perhaps we can just move combinePowerString to somewhere else common, and remove this file?
   public constructor( numberProperty: TReadOnlyProperty<number | null>, displayRange: Range, providedOptions?: NumberDisplayOptions ) {
     super(
       numberProperty,