search

cyprieng / swagger-parser

Give useful informations about your swagger files
MIT License
62 stars 59 forks source link

Fixes for #31, #32, #33 #34

Closed crudo10 closed 7 years ago

beanqueen commented 7 years ago

i think you need to update your PR, since "There are no new changes to show."

cyprieng commented 7 years ago

The tests are failing due to lint error: https://travis-ci.org/Trax-air/swagger-parser/jobs/178064560

cyprieng commented 7 years ago

Fix #31 Fix #32 Fix #33

cyprieng commented 7 years ago

Line 872 and 875 are too long, max length is 120

cyprieng commented 7 years ago

Good the tests are green :smile: Can you add one test for each of your fix (eg for each of your issues) ?

crudo10 commented 7 years ago

I am not sure I am familiar with the test strategy. Do I need to modify some data (swagger) sample to introduce test cases that caused bugs? Or I can do that at UT level?

cyprieng commented 7 years ago

I think the best option is to call the functions you just fix with args that was failing before your test, and check that is working now. You can put this either in an already existing test case (function in this file: https://github.com/Trax-air/swagger-parser/blob/master/tests/test_swagger_parser.py) if one is already testing the function, or create a new test case.

beanqueen commented 7 years ago

we might want to consider some refactoring of the code to reduce the number of indentation levels and enhance readability

cyprieng commented 7 years ago

Sorry for the really long delay. I will check this soon :smiley:

philippeluickx commented 7 years ago

Running into some errors which would be solved with this PR. Would be awesome to get this to master!

flavianh commented 7 years ago

I resolved the conflict and kept the logging line for safety

flavianh commented 7 years ago

@philippeluickx I'm merging this. @cyprieng I'll fix the tests in a next PR to drop python2.6 support. I'll make a major version bump then