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

Fixed imEntity value correctly in demo.html based on user selection #31

Closed

Conversation

Sakshisrivastava413
Copy link
Member

Changed the demo.html imEntity dataToInitialiseToolWith based on the class, accept chosen in the setup by the user and accordingly setting value from the constant id array.
Refer to PR #29

@heralden
Copy link
Member

Sorry for taking so long to review!

Could you run eslint and prettier? It seems to complain about if(val == 'id') not having a space after the if, and using ==.

 275:5   error  Insert `·`                           prettier/prettier

  275:10  error  Expected '===' and instead saw '=='  eqeqeq

Also a quick question: I see <%= format %> and <%- value %> (notice the - instead of =). Is this a typo?

@Sakshisrivastava413
Copy link
Member Author

Seems weird as I am getting eslint error in node_modules instead of that you mentioned above.

I see <%= format %> and <%- value %> (notice the - instead of =). Is this a typo?

yes, correct is = although it's working fine with - too. Will change that part to follow one standard everywhere.

@akshatbhargava123
Copy link
Member

Seems weird as I am getting eslint error in node_modules instead of that you mentioned above.

I also get the eslint error in something related to observer module from node_modules.

@heralden
Copy link
Member

I tried cloning Sakshi's fork into a fresh directory, then running npm install && npm test. I still get the same error as Travis CI.

> @intermine/[email protected] pretest /home/regen/prog/work/foo
> eslint .


/home/regen/prog/work/foo/generators/app/index.js
  275:5   error  Insert `·`                           prettier/prettier
  275:10  error  Expected '===' and instead saw '=='  eqeqeq

✖ 2 problems (2 errors, 0 warnings)
  1 error and 0 warnings potentially fixable with the `--fix` option.

npm ERR! Test failed.  See above for more details.

If you're getting a different error, can you paste that?

@Sakshisrivastava413
Copy link
Member Author

Running tests throws the lint errors you mentioned. But while doing commit or running npm run precommit is throwing error in node_modules
Screenshot 2020-08-12 at 1 49 17 PM

@heralden
Copy link
Member

Alright, that is a separate error caused by something else. I will create an issue for it.

Can you please fix the eslint errors the PR introduces, then run git commit --no-verify and git push --no-verify?

@heralden
Copy link
Member

Looks like Travis is failing on npm test with a different error that looks related to this PR.

 FAIL  __tests__/app.js
  generator-bluegenes-tool:app
    ✕ creates files (11ms)

  ● generator-bluegenes-tool:app › creates files

    TypeError: Cannot read property '0' of undefined

      135 |       reactReq: this.props.reactReq,
      136 |       classes: getFirstElement(this.props.classes),
    > 137 |       format: this.props.accepts[0],
      138 |       value: getValue(this.props.accepts[0])
      139 |     });
      140 |

      at Object.<anonymous>.module.exports.writing (generators/app/index.js:137:15)
      at Object.<anonymous> (node_modules/yeoman-generator/lib/index.js:399:25)
      at node_modules/run-async/index.js:49:25
      at node_modules/run-async/index.js:26:19
      at self.env.runLoop.add.completed (node_modules/yeoman-generator/lib/index.js:400:11)

@Sakshisrivastava413 Could you check what's wrong? Might just be that the tests have to be adjusted to your change.

@heralden
Copy link
Member

I'm changing demo.html to use imjs instead of hardcoded object IDs (#30), so this will require a different approach to solve.

@heralden heralden closed this Feb 26, 2021
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.

3 participants