-
-
Notifications
You must be signed in to change notification settings - Fork 633
Chore: eslint per track, loose rules #636
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
Conversation
|
||
/** | ||
* This node js executable reads in the top-level package.json, babel.config.js | ||
* and the special .eslintrc.track.json and copies them to each exercise. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can the Makefile do this? Since it already copies package.json
, ASSIGNMENT
and TEST_FILE
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check the other PR to see where this is going.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(so yes, but I rather have ALL functionality in Make or ALL in node/sh, not all over the place. I don't know how to parse json in Make, remove a key and do a comparison)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The point is that we're forcing contributors to learn and install Make when everything else is in JavaScript -- and now I'll have to install even more dependencies (like jq
) which are NOT managed inside package.json. We're using npm as package manager and to get the dependencies in but then use another system to run scripts?
], | ||
]; | ||
|
||
module.exports = { presets }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry if I am missing something, why do we need to separate babel config from package.json?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because we shouldn't store it in package.json just like we shouldn't store eslint rules in package.json. Either move both out or neither.
@SleeplessByte I think we should split this into separate PRs for:
We can do it one after the other to avoid conflicts. What do you think? |
@tejasbubane eslint per track without upgrade to new version could work separately; so yes I can do these three if that's what you prefer 👍 as long as we merge them right after eachother 🥇 |
@SleeplessByte If you are busy (with the track reordering) let me know if I can pick up something from here. |
@tejasbubane I have this planned for tomorrow :) |
Fixes #620, by adding track-only eslint
Fixes #589, by adopting eslint:recommended for students (track-only)
Fixes #502 (this is handled by git)
Fixes #44
Workaround for maintainers on #421 by adding a script to copy the package.json to all exercises