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

add github actions test workflow #187

Merged
merged 2 commits into from
May 21, 2020

Conversation

dhimmel
Copy link
Contributor

@dhimmel dhimmel commented May 21, 2020

Haven't test this yet... will update with whether it works.

I find GitHub actions is a bit slicker than Travis and is nicely integrated with GitHub. If we merge this, could then remove Travis.

@dhimmel
Copy link
Contributor Author

dhimmel commented May 21, 2020

looks to be working https://github.com/dhimmel/csl-schema/runs/697715882

will remove Travis since not much point in having both services active

@dhimmel
Copy link
Contributor Author

dhimmel commented May 21, 2020

@bdarcus care to review?

@bdarcus bdarcus merged commit 991d8f6 into citation-style-language:master May 21, 2020
@bdarcus
Copy link
Member

bdarcus commented May 21, 2020

I don't really know anything about this, but I merged, figuring if there's some minor problem, we can fix it :-)

@bdarcus
Copy link
Member

bdarcus commented May 21, 2020

And of course, as soon as I say that, I get this:

https://github.com/citation-style-language/schema/actions/runs/111868243

@dhimmel
Copy link
Contributor Author

dhimmel commented May 21, 2020

And of course, as soon as I say that, I get this:

haha. it's trying to resolve some non-existent URL (log):

url = 'https://github.com/citation-style-language/schema/raw/master/date-variable'

I'll look into it

@dhimmel dhimmel deleted the actions branch May 21, 2020 23:48
@dhimmel
Copy link
Contributor Author

dhimmel commented May 22, 2020

I think the problem has to do with how we define date-variable:

"id": "date-variable",

"$ref": "date-variable"

In draft-07, it seems like this might need to be placed in a definition and referenced like { "$ref": "#/definitions/date-variable" }. I think @dsifford dealt with this in #161. What we want is the most minimal change to get our definitions working.

@bdarcus
Copy link
Member

bdarcus commented May 22, 2020 via email

@dhimmel
Copy link
Contributor Author

dhimmel commented May 22, 2020

Oh, so the GitHub action did work correctly?

I think so, the error message is not super clear, but I think the issue is that the upgrade to draft-07 (and draft-06) in 7179185 has some problems. These problems did not surface until we added the test in this pull request, since previously no tests were checking for whether definitions worked.

do you think you could submit another PR to fix the bug it uncovered?

probably not soon. I don't have much experience with JSON schemas. @dsifford might be able to take part of #161 to fix it without too much hassle.

@bdarcus
Copy link
Member

bdarcus commented May 22, 2020

That validators aren't very helpful.

@dsifford might be able to take part of #161 to fix it without too much hassle.

That'd be awesome if he could; I'm struggling with this.

@bdarcus
Copy link
Member

bdarcus commented May 22, 2020

So the problem may be simply that the pattern that's referenced is in the wrong place.

@dsifford
Copy link

Let me know if you aren't able to get it sorted @bdarcus and I'll have a look later this afternoon.

@dhimmel
Copy link
Contributor Author

dhimmel commented May 22, 2020

Okay, I opened a dedicated issue for this at #188

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.

3 participants