-
Notifications
You must be signed in to change notification settings - Fork 2
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
code review #52
Comments
PhET code-review checklistBuild and Run Checks
Strings
Images
Repository structure
my-repo/
assets/
audio/
license.json
doc/
model.md
implementation-notes.md
images/
license.json
js/
my-repo-config.js
my-repo-main.js
.gitignore
my-repo_en.html
my-repo-strings_en.json
Gruntfile.js
LICENSE
package.json
README.md For a common-code repository, the structure is similar, but some of the files and directories may not be present if the repo doesn’t have audio, images, strings, or a demo application.
my-repo/
js/
common/
model/
view/
custom
model/
view/
introduction
model/
view/
my-repo-config.js
my-repo-main.js
Coding conventions
Documentation
Common Errors
Organization, Readability, Maintainability
Performance, Usability
// Cap large dt values, which can occur when the tab containing
// the sim had been hidden and then re-shown
dt = Math.min( 0.1, dt ); Memory Leaks
|
Dev version published prior to making any significant changes: |
I skipped 2 of the checklist items, which didn't seem worth devoting the time until the sim is feature complete. Instead, I created issues for these: #80 - is performance acceptable?
#79 - check for memory leaks
Code review completed, all issues created are linked to this issue above. Assigning to @ariel-phet so he can take a quick "survey" of the state of things before closing. An item in the checklist preceded by ❌ indicate that the sim did not pass review for that item, and one or more issues was created. |
I also should note that the sim is in overall good shape, and @schmitzware did a nice job of identifying the work that's still to be done. |
@pixelzoom thanks for marking the checklist and giving the overview. Agreed on the two skipped items. Closing |
@ariel-phet asked me to code review unit-rates. OK if this happens after @schmitzware has finished his contract. The idea is that I will review and just make any changes that need to be done, rather than creating issues and having @schmitzware address them.
@schmitzware please assign back to me if the sim is ready for review before your contract ends.
The text was updated successfully, but these errors were encountered: