-
-
Notifications
You must be signed in to change notification settings - Fork 43
Props and sync #549
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
Props and sync #549
Conversation
|
@eduarddrenth Thanks for the updated PR! I checked out your branch and tried to build the package according to the README, using |
|
p.s. Normally I believe we could've relied on Travis to perform these tests for us on PRs, but I gather from discussion in Slack that the eXist-db GitHub organization exceeded Travis's free allotment of tests, and there's no solution in sight. So for now I think this means we've got to perform tests locally. |
I'll take a look, a pitty this one because Oxygen says all is well |
|
validates now using mvn validate |
|
@eduarddrenth Great, I can confirm validation now passes. |
|
@eduarddrenth I wonder if there might be any easier method for making this feature available to users of eXist. For example, for users accustomed to building eXist from source via maven, is there something that these users could add to eXist's pom.xml to ensure the proper artifacts are retrieved? |
You mean this? |
|
don't know why the build fails. Something changed in build process? |
|
@eduarddrenth These tests are failing: Is it possible you have some empty section headings now, or that they are missing an id attribute? The failing tests is this one (I think) - https://github.com/eXist-db/documentation/blob/master/src/main/xar-resources/modules/test-suite.xql#L57 |
So I saw yes, but I did not change a thing except for a new version number of the lib. No emty section headers or other empty things. Is this test new? This one fails:
Looks like CDATA sections are not allowed anymore? |
|
@eduarddrenth I am not sure how new the tests are, they were added by @duncdrum. The tests were not being run on CI until recently when they were reinstated. |
joewiz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed during this week's Community Call, all requests have been addressed, and all tests are passing.
|
Congrats on your first merged pull request! |
|
@eduarddrenth Thank you for contributing this article! |
|
Nice! |
sync fixed and tested
bootstrap updates reverted (building documentation probably updates boot strap, unwanted I think)