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.

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

Initial Review #1

Closed
samreid opened this issue Oct 24, 2014 · 1 comment
Closed

Initial Review #1

samreid opened this issue Oct 24, 2014 · 1 comment
Assignees

Comments

@samreid
Copy link
Member

samreid commented Oct 24, 2014

@AshrafSharf has been doing excellent work on porting the simulation, and it is time to have the PhET developers take a closer look. @samreid & @jbphet will take a closer look, and follow roughly the following steps:

  1. We'll post a development version "1.0.0-dev.4" to have a snapshot of the version for which we began testing
  2. We'll perform "black box" testing -- that is, looking at the behavior of the simulation by itself (without looking at the code). As part of this, we will use the development query parameters such as ?dev, ?fuzzMouse ?ea and ?showPointerAreas. We should also hack string.js to double all of the internationalized strings to make sure they are translatable and that the layout supports longer strings. We can also use phetAllocation.js to understand the memory behavior in the sim. We can check against the jshint.
  3. Next we can take a closer look at the code itself, looking for issues regarding readability, maintainability, modularity, etc. This code review may also reveal potential functional problems in the simulation. As we look at the code, we may make minor changes (renaming/refactoring/adding documentation/etc) which we will commit with this issue number, so they will appear linked in this issue. Larger issues we will create issues for, and we will also link them to this issue and mark with a "code-review" label for easy tracking.
  4. Another step will be to have PhET's QA team test the simulation to see if they discover any problems on our many supported platforms. This will be done after several iterations of steps 1-3.
jbphet added a commit that referenced this issue Oct 24, 2014
… some whitespace and removed some blank lines. Other than that, the code adhered to PhET's code style standards. See #1
jbphet added a commit that referenced this issue Oct 27, 2014
@jbphet
Copy link
Contributor

jbphet commented Dec 3, 2014

I'm going to close this issue and track the remaining code review comments under #29, and will log any other functional issues as separate tickets.

@jbphet jbphet closed this as completed Dec 3, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants