Skip to content

Conversation

@IvanGoncharov
Copy link
Contributor

Implement based on comments from #404

@IvanGoncharov
Copy link
Contributor Author

It passing all JS test, but fails somewhere inside Scala tests.
I don't have any Scala knowledge to fix it :(

@webron
Copy link
Member

webron commented Jul 16, 2015

@IvanGoncharov - thanks, I'll review it and the tests. If the change is good, I'll work on the tests. It'll take a while though.

@IvanGoncharov
Copy link
Contributor Author

I review Scala tests and they are very simple.
Is there are any reasons why they implemented in Scala?
They could be easily replaced with 30-40 lines in JS.
It could be easier than fix bugs in abandoned library:
https://github.com/fge/json-schema-validator#this-project-is-looking-for-a-new-maintainer

@webron
Copy link
Member

webron commented Jul 16, 2015

Knowing the library's author, I also know the project is not abandoned.

@IvanGoncharov
Copy link
Contributor Author

So how about moving these test into https://github.com/swagger-api/swagger-spec/blob/master/test/validate.js ?
I can create separate PR.

@webron
Copy link
Member

webron commented Jul 16, 2015

Sure, but unfortunately it would take me a while to review it.

@IvanGoncharov
Copy link
Contributor Author

I understand.
I will create separate PR and link it to this.

@webron
Copy link
Member

webron commented Jul 16, 2015

Thanks!

@webron
Copy link
Member

webron commented Jul 30, 2015

@IvanGoncharov - any update on this?

@IvanGoncharov
Copy link
Contributor Author

@webron Just started migrated tests.
Problem that for some reason I can't run even vanilla tests on my machine :(
I will create separate issue with WIP status.

@webron
Copy link
Member

webron commented May 20, 2016

@IvanGoncharov - closing this for now. Really trying to minimize changes to things related to 2.0.

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.

2 participants