Skip to content
This repository has been archived by the owner on Aug 4, 2023. It is now read-only.

Add support for OOP in controllers. #402

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

pixelshaded
Copy link

No description provided.

@whitlockjc
Copy link
Member

Did you run gulp to see that all the lint checks passed and unit tests ran?

@pixelshaded
Copy link
Author

pixelshaded commented Jul 1, 2016

I went back and ran npm install. npm test fails with

'.' is not recognized as an internal or external command,

created a gulp script and it ran all the tests. 34 failed. 33 of them were

-'RUNNING_SWAGGER_TOOLS_TESTS' is not recognized as an internal or external command.

and test 7) CLI Global commands convert invalid resourceListing argument (non-existent file) failed expecting true but getting false.

@whitlockjc
Copy link
Member

Maybe there are some developer dependency changes that haven't been synced into the project but both gulp and npm test run fine here. You shouldn't had needed to create a gulpfile.js. It seems TravisCI thinks things are fine too: https://travis-ci.org/apigee-127/swagger-tools/builds/141469540

@Nepoxx
Copy link

Nepoxx commented Aug 4, 2016

Any updates on this? AFAIK this is the only way to use swagger-tools with Typescript right?

@whitlockjc
Copy link
Member

No update. Once I asked the OP to run tests, I never got a response back.

@pixelshaded
Copy link
Author

pixelshaded commented Aug 4, 2016

The swagger-tools router doesn't behave as I suspected so I stopped using it (which made this PR moot for me). We use something similar to the one I posted in the issue: #284 (comment).

@Nepoxx This was an issue related to using classes. Typescript was brought up because classes are popular in it. You can still use Typescript with swagger-tools, as long as your exports are plain objects.

@whitlockjc
Copy link
Member

Well, merging this PR isn't an issue so it should behave as you expected once that's done. I was waiting on a json-refs release to merge this but I can expedite it if that helps.

@Nepoxx
Copy link

Nepoxx commented Aug 8, 2016

I'm using a plain object right now (because OOP wasn't working), so it's not a big deal.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants