phetsims / blackbody-spectrum

"Blackbody Spectrum" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
1 stars 3 forks source link

quick code review #15

Closed pixelzoom closed 5 years ago

pixelzoom commented 6 years ago

This sim is being considered for a student project. I'll do a "quick" (not formal or complete) code review to evaluate the state of this sim, then report back to @ariel-phet.

pixelzoom commented 6 years ago

This sim is in good shape. I did a minor amount of cleanup in the above commits, and created 1 related issue (#17).

This is a relatively small sim (10 files, ~1300 lines of code), and it looks feature complete (or close to it). Unless there are significant additional features to add, there's not much in the way of interesting development here. It looks to me like it's close to being done, and just needs someone to polish it off. Something to keep in mind when deciding whether this sim is appropriate for a student project...

Back to @ariel-phet to review results of code review.

ariel-phet commented 6 years ago

@pixelzoom thanks. Seems more like a good starting project for a new internal intern. Clean this up, and get it published.

pixelzoom commented 6 years ago

1/25/18 dev meeting:

@ariel-phet said that most of the work to be done is cosmetic and pixel polishing. @pixelzoom said that there is some additional code cleanup that would be worthwhile. @jbphet asked me to add notes about anything else that I recommend cleaning up in the code.

pixelzoom commented 6 years ago

@jbphet asked me to add notes about anything else that I recommend cleaning up in the code.

@jbphet I recommend cleaning up #17, #18, #19 before turning this over to an intern.

jbphet commented 6 years ago

One of the three issues listed in the comment above has been addressed, but the other two have not been. Work on this sim is on hold for a while, so I'm unassigning until work resumes.

arnabp commented 5 years ago

Supplanted by primary code review: https://github.com/phetsims/blackbody-spectrum/issues/33