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

chipper should be lint-able #92

Closed
pixelzoom opened this issue Feb 6, 2015 · 16 comments
Closed

chipper should be lint-able #92

pixelzoom opened this issue Feb 6, 2015 · 16 comments

Comments

@pixelzoom
Copy link
Contributor

This issue was moved out of #62.

chipper contains JS code, and we currently have no build support for linting it. The JS code includes:

• chipper/requirejs-plugins/ - A quick look at these files shows that they will fail lint. (Hence the 'bug' label for this issue.) Some files are missing 'use strict'. And there may be other issues that we don't know about.

• chipper/grunt/ - implementation of grunt tasks, is not currently linted.

• And in phetsims/phetcommon#23, we decided to consolidate 'check' scripts into initialize-globals.js and relocate under chipper.

The easiest (and most consistent) solution would be to put all JS code under chipper/js/, so that the existing grunt lint works. Another alternative would be to add something to package.json that enumerates the JS-source directories. My preference would be to put all JS source under js/.

Assigning to @samreid for comment, and since he's handling #62.

@pixelzoom
Copy link
Contributor Author

@samreid @jonathanolson @jbphet Does anyone mind if I move all chipper JavaScript files under chipper/js, so that it will be lint-able?

@samreid
Copy link
Member

samreid commented Feb 10, 2015

Are you planning to change the *-config.js files for each simulation? They currently reference: paths like ../../chipper/requirejs-plugins/image. Also, have you checked that we have the capability to make all of our chipper JS files lint-free? There may be external constraints that could make it difficult.

@samreid
Copy link
Member

samreid commented Feb 11, 2015

I agree we should try to move things under chipper/js, keeping the directory structure as much as possible. For instance we will have:

chipper/js/requirejs-plugins/
chipper/js/grunt/
etc.

Like I mentioned above, this will affect all config files, and it is worth briefly checking (by copy/paste to the js directory) that our other js files will have a chance of passing hint. @pixelzoom feel free to proceed if you wish or let me know if you'd like me to do so.

@samreid samreid assigned pixelzoom and unassigned samreid Feb 11, 2015
@jonathanolson
Copy link
Contributor

Curiously due to phetsims/build-a-molecule#71, BAM is now linting chipper files under js/. Does it make sense for all sims to have chipper in their dependencies like that?

@pixelzoom
Copy link
Contributor Author

@samreid asked:

have you checked that we have the capability to make all of our chipper JS files lint-free?

No, I haven't. But if it's not possible to make some chipper code conform to PhET's jshint standards, I think it would be preferable to disable specific warnings (e.g. /* jshint -W034 */) rather than skip linting of foundation code.

@jonathanolson asked:

Does it make sense for all sims to have chipper in their dependencies like that?

chipper should be lint-able independently of sims or other repos.

@pixelzoom
Copy link
Contributor Author

@samreid asked:

Are you planning to change the *-config.js files for each simulation?

Yes. I'm proposing to move chipper/requirejs-plugins to chipper/js/requirejs-plugins, and change all sim *-config.js to reflect the new location.

Also proposing to move chipper/grunt to chipper/js/grunt. Then fix paths in chipper/js/grunt/Gruntfile.js, and in all sim top-level Gruntfile.js files.

@pixelzoom
Copy link
Contributor Author

See #95, #96 and #97 for current lint errors in chipper.

pixelzoom added a commit to phetsims/balancing-chemical-equations that referenced this issue Feb 12, 2015
pixelzoom added a commit to phetsims/balancing-act that referenced this issue Feb 12, 2015
pixelzoom added a commit to phetsims/curve-fitting that referenced this issue Feb 12, 2015
pixelzoom added a commit to phetsims/simula-rasa that referenced this issue Feb 12, 2015
pixelzoom added a commit to phetsims/blackbody-spectrum that referenced this issue Feb 12, 2015
pixelzoom added a commit to phetsims/balloons-and-static-electricity that referenced this issue Feb 12, 2015
pixelzoom added a commit to phetbrand/brand that referenced this issue Feb 12, 2015
pixelzoom added a commit to phetsims/assert that referenced this issue Feb 12, 2015
pixelzoom added a commit to phetsims/acid-base-solutions that referenced this issue Feb 12, 2015
pixelzoom added a commit to phetsims/faradays-law that referenced this issue Feb 12, 2015
pixelzoom added a commit to phetsims/function-builder that referenced this issue Feb 12, 2015
pixelzoom added a commit to phetsims/arithmetic that referenced this issue Feb 12, 2015
pixelzoom added a commit to phetsims/area-builder that referenced this issue Feb 12, 2015
pixelzoom added a commit to phetsims/griddle that referenced this issue Feb 12, 2015
GogoBeer added a commit to GogoBeer/rN-INCOMING-CALl that referenced this issue Feb 24, 2023
Q7Uyec added a commit to Q7Uyec/AUTOCHIPMOVIES that referenced this issue Mar 3, 2023
HomeJoyer added a commit to HomeJoyer/smodev-pORTFOLIO that referenced this issue Mar 11, 2023
Hotelstrand added a commit to Hotelstrand/A-TodoList-A-TodoList that referenced this issue Mar 14, 2023
BlackRockie added a commit to BlackRockie/pokemon101 that referenced this issue Apr 17, 2023
Vertigogo added a commit to Vertigogo/staff_list that referenced this issue Apr 17, 2023
typerform added a commit to typerform/Tatiana-android-apP that referenced this issue Apr 24, 2023
hecha2 added a commit to hecha2/Novel-to-Sound that referenced this issue Apr 24, 2023
YesRealLife added a commit to YesRealLife/JEONGHAE10.IO that referenced this issue May 6, 2023
Yoimor added a commit to Yoimor/matchscorewebsite that referenced this issue May 6, 2023
reimg added a commit to reimg/ml_proj_temp that referenced this issue May 7, 2023
Qibiui added a commit to Qibiui/microverseproject2 that referenced this issue May 7, 2023
15196680454 added a commit to 15196680454/Admin-Symfony that referenced this issue May 10, 2023
Thrupple added a commit to Thrupple/MLwithGoExamples that referenced this issue Nov 14, 2023
TrueViso added a commit to TrueViso/neuroML-education-resources that referenced this issue Jun 3, 2024
The-wang-qiao added a commit to The-wang-qiao/code-review-augre that referenced this issue Jun 3, 2024
lolocake added a commit to lolocake/Numerics-Enhanced that referenced this issue Jun 3, 2024
brendalo added a commit to brendalo/awesome-deepnetts-engine that referenced this issue Jun 3, 2024
LookOutGap added a commit to LookOutGap/LookOutGap-NLPAssistant that referenced this issue Jun 3, 2024
designhak added a commit to designhak/ml-for-material-science-tutorials that referenced this issue Jun 3, 2024
jujusti added a commit to jujusti/diverse-ml-task-queue that referenced this issue Jun 4, 2024
VoxAtx added a commit to VoxAtx/watchtower-ai-voxatx that referenced this issue Aug 7, 2024
AIwaifus added a commit to AIwaifus/http-anomaly-detection-seq2seq that referenced this issue Aug 8, 2024
LaunchGemini added a commit to LaunchGemini/ImageAnnotatorPro that referenced this issue Sep 7, 2024
Mikihail pushed a commit to Mikihail/pro-object-detection-2d that referenced this issue Sep 20, 2024
aran085 added a commit to aran085/Unified-Multi-lesion-Segmentation that referenced this issue Sep 27, 2024
FilledOfCode added a commit to FilledOfCode/Larkord that referenced this issue Nov 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants