Skip to content

Add: Eslint in root files/folders #177

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

Merged
merged 5 commits into from
Mar 17, 2020
Merged

Conversation

vibhorgupta-gh
Copy link
Contributor

Fixes #166
Check issue comments for description

@vibhorgupta-gh
Copy link
Contributor Author

@umeshp7 added linting, however there are a lot errors that cause a process exit. Shall I push a commit fixing them as well?

@umeshp7
Copy link
Contributor

umeshp7 commented Feb 27, 2020

@VibhorCodecianGupta Yes, it would be good to have those fixes as well.

@vibhorgupta-gh
Copy link
Contributor Author

@umeshp7 fixed lint errors except one, test/codegen/structure.test.js line 44 violates max length, not sure how you'd like to fix that (break the string in newline?). Also, npm run test fails at newman tests for basicCollection, it gives a ENOENT for all the tests in the block

@shreys7
Copy link
Member

shreys7 commented Feb 28, 2020

@VibhorCodecianGupta you can split the line 44 into two strings, the second one on a new line. For the ENOENT failure, can you share the exact error? and for which codegen it is failing?

@umeshp7
Copy link
Contributor

umeshp7 commented Mar 1, 2020

Some tests are failing here: https://travis-ci.com/postmanlabs/postman-code-generators/builds/150958863#L2505

@VibhorCodecianGupta

@umeshp7
Copy link
Contributor

umeshp7 commented Mar 2, 2020

The test that is failing is due to a duplicate copy of a dependency postman-collection. Remove it from devDependencies and it should be pass. @VibhorCodecianGupta

@umeshp7
Copy link
Contributor

umeshp7 commented Mar 9, 2020

@VibhorCodecianGupta any updates on this? We'll have to close this soon due to inactivity.

@vibhorgupta-gh
Copy link
Contributor Author

vibhorgupta-gh commented Mar 10, 2020

@shreys7 @umeshp7 so no-multi-str is also enabled in .eslintrc, which makes splitting strings in multiple lines erroneous. I think you should follow either of the length/multi string check, suggestions?
Also I had my midterms, I'll resolve this issue asap and push

Also, the ENOENT tests are failing because npm run test checks for the presence of .eslintrc in the path ${CODEGEN_ABS_PATH}/.eslintrc. Now for most packages the eslintrc exits in ${CODEGEN_ABS_PATH}/test/.eslintrc while for some of them I couldn't find an rc file at all. Should I create an eslintrc for every codegen and place it inside test/ ? That way the text can be fixed

@vibhorgupta-gh
Copy link
Contributor Author

@umeshp7 @shreys7 update, rebasing will resolve enoent failures.

@shreys7 @umeshp7 so no-multi-str is also enabled in .eslintrc, which makes splitting strings in multiple lines erroneous. I think you should follow either of the length/multi string check, suggestions?

This issue still remains though. How to proceed?

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.

Adding eslint in system tests
3 participants