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

lint should check all relevant js files #1425

Merged
merged 1 commit into from Jul 6, 2018

Conversation

tristanmkernan
Copy link
Collaborator

issue #1422

this PR adds the js files in the src/ directory (subdirectories were already included previously) to the lint check and resolves the relevant errors.

once again some eslint config changes were required because i'm unable to fix them at the present time:

  • no-eval: eval is used at one point in the code
  • no-new involves side effects and new - this is used for Drops

@ktiedt
Copy link
Collaborator

ktiedt commented Jul 5, 2018

The evals should be simple replaces... just remove the eval() they appear to be just basic operations?

@ktiedt
Copy link
Collaborator

ktiedt commented Jul 5, 2018

Can you be more specific on the new one? I dont see drops being created without new Drop(...) which I believe that pertains to?

@tristanmkernan
Copy link
Collaborator Author

The evals should be simple replaces... just remove the eval() they appear to be just basic operations?

I don't know, I don't want to touch that code right now.

Can you be more specific on the new one? I dont see drops being created without new Drop(...) which I believe that pertains to?

right, new Drop(..) is used for side effects instead of for creating an object. the linter rightly calls it out as bad code

@ktiedt
Copy link
Collaborator

ktiedt commented Jul 5, 2018

Ahh because Drop() is assigning itself to the hex instead of storing the new Drop() value... Ugh.

@DreadKnight DreadKnight merged commit e1cbf91 into FreezingMoon:master Jul 6, 2018
CyberBishop pushed a commit to CyberBishop/AncientBeast that referenced this pull request Apr 20, 2023
lint should check all relevant js files, fixes FreezingMoon#1422
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

Successfully merging this pull request may close these issues.

None yet

3 participants