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

Use continuous integration and define full suite of tests to validate all PR's #199

Closed
4 tasks done
cholmes opened this issue Aug 24, 2018 · 8 comments
Closed
4 tasks done
Assignees
Labels
prio: must-have required for release associated with
Milestone

Comments

@cholmes
Copy link
Contributor

cholmes commented Aug 24, 2018

Thanks @scisco for doing the bulk of the work on this, and @JustinWP for having the same idea and getting it set up with Travis (may want to use some of his checks in CircleCI). He's got CircleCI set up on master and dev.

Right now it's not doing much, but is working. We likely don't want to do the full validation until we merge the major PR's and update the examples all in one go, instead of having each json schema PR update all the examples just for their change.

  • set up circleci
  • check the api spec creation Add ci checks for api [WIP] #183
  • update all the examples for the new spec.
  • turn on CI for json schema checks of items and catalog samples.
@cholmes cholmes added the prio: must-have required for release associated with label Aug 24, 2018
@cholmes cholmes added this to the 0.6.0 milestone Aug 24, 2018
@m-mohr
Copy link
Collaborator

m-mohr commented Aug 31, 2018

The CI should also make sure that the generated API specs are up-to-date, see my latest comment in #208.

Additionally, I restructured and fixed several things regarding validation, see PR #230 .

@cholmes
Copy link
Contributor Author

cholmes commented Aug 31, 2018

Yes, we need that. That's what I was trying to say in the checkbox above on 'check the api spec creation'. I thought #183 would do that, but it looks like @scisco is going to take another approach.

@cholmes
Copy link
Contributor Author

cholmes commented Oct 8, 2018

Discussed on a call today - it would be good to have a validation tool that can reference remote schemas, which isn't in place now. May be good to use the validation tool @jbants is working on. James, if you could link to the python validator from here that could help @matthewhanson get it set up.

@jbants
Copy link
Collaborator

jbants commented Oct 8, 2018

https://github.com/sparkgeo/stac-validator
@matthewhanson let me know if you want some help.

@cholmes
Copy link
Contributor Author

cholmes commented Oct 12, 2018

We also discussed that it would be ok to do this work after RC1, since it hopefully doesn't change the spec at all. If spec changes are needed we'd cut an RC2, but most likely we'd just get the validation working and maybe update examples and release 0.6.0

@cholmes
Copy link
Contributor Author

cholmes commented Oct 16, 2018

Changed to 0.6.0 Would be good to update the CI to use the stac validator (as I think it can pull in remote references), and ensure all the examples are up to date, and have that CI in place for all future dev development (so people who make PR's that change the schema need to update the examples too). And we should ensure the OpenAPI doc combining stuff is all working.

Also consider doing formatting. Like #298 on the examples. We could even consider it for the markdown formatting as well... Though that should probably wait till 0.7.0 so we don't have tons of changes after RC1

@m-mohr
Copy link
Collaborator

m-mohr commented Oct 17, 2018

As said in #298, it would be useful to write a script that combines the following tasks:

  • Check the style of examples (excluding the examples included in the markdown files)
  • Check the style of JSON schemas
  • Check the validity of JSON schemas
  • Check the validity of examples based on the JSON schemas
  • Generate API documents from fragments (for api-spec and extensions)

Not sure about Markdown formatting though.

That script should be running on the CI and should also be usable by the user so that before every PR they can make sure everything is valid with a single call. Currently it includes a ton of checks that need to be done separately.

We may need separate issues for them and solve this in steps, but all of this should be available eventually.

@mojodna mojodna self-assigned this Oct 18, 2018
@cholmes cholmes assigned jbants and unassigned matthewhanson Oct 23, 2018
@m-mohr m-mohr modified the milestones: 0.6.0, 0.7.0 Nov 7, 2018
@cholmes cholmes assigned matthewhanson and unassigned mojodna and jbants Apr 22, 2019
@cholmes
Copy link
Contributor Author

cholmes commented Apr 22, 2019

@matthewhanson - assigning to you as you were going to make some progress on this. Feel free to redefine to be a smaller scope, and put any more todo's in their own issues.

#434 and #436 are related

@cholmes cholmes closed this as completed May 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
prio: must-have required for release associated with
Projects
None yet
Development

No branches or pull requests

5 participants