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 latest Saxon 9.6.0-10 for ci tests #68

Merged
merged 3 commits into from Jan 16, 2017
Merged

Use latest Saxon 9.6.0-10 for ci tests #68

merged 3 commits into from Jan 16, 2017

Conversation

tofi86
Copy link

@tofi86 tofi86 commented Jan 15, 2017

Saxon 9.6 series got an update in December 2016

@cirulls
Copy link
Member

cirulls commented Jan 15, 2017

Thanks for your pull request.

Using Saxon 9.6.0-7 instead of 9.6.0-10 version was actually a choice made in #43:

Although the latest 9.6 is 9.6.0.10, I chose 9.6.0.7 just because it's the one bundled currently in oXygen.

We can trigger tests for all three versions of Saxon, that would not be a problem. The only drawback is that running the test suite via AppVeyor for the Windows environment will take a bit longer (Travis doesn't have this problem as it runs tests in parallel).

I assign the review to @AirQuick as he (or she?) did the initial implementation for AppVeyor.

@tofi86
Copy link
Author

tofi86 commented Jan 15, 2017

Using Saxon 9.6.0-7 instead of 9.6.0-10 version was actually a choice made in #43:

oh, okay... yeah, well, so it's upt to you and @AirQuick whether you want to accept this, test three versions or decline the PR...

@AirQuick
Copy link
Member

While 9.6.0.8 was released in 2015-12, oXygen still sticks to 9.6.0.7 to date. So I (he) fancied 9.6.0.7 might be widely used. No other specific reason. It's ok to just move on to 9.6.0.10.

By the way, is Syncro Soft aware of this repository? Will they sooner or later consider updating XSpec in oXygen based on this repository?

@tofi86
Copy link
Author

tofi86 commented Jan 15, 2017

By the way, is Syncro Soft aware of this repository? Will they sooner or later consider updating XSpec in oXygen based on this repository?

We (@paginagmbh) are an oXygen partner. I can give @georgebina a shout and will also write an email to their support team.

@cirulls
Copy link
Member

cirulls commented Jan 15, 2017

By the way, is Syncro Soft aware of this repository? Will they sooner or later consider updating XSpec in oXygen based on this repository?

Funny that you mention this, straight after the release I sent a message to this Oxygen Forum asking to try out the latest release and possibly integrate in the future. I'm waiting for the message approval, I'll post the link here when it's visible so you can follow their reply.

I don't mind testing for all three versions of Saxon as I would like to keep the Saxon version used by Oxygen. Which option would you prefer:

  1. Run tests with Saxon 9.7.0-14, 9.6.0-7, and 9.6.0-10 (aka request changes to the pull request)
  2. Run tests with Saxon 9.7.0-14 and 9.6.0-10 (aka accept the pull request)
  3. Run tests with Saxon 9.7.0-14 and 9.6.0-7 (aka reject the pull request)

I'm fine with option 1. @tofi86 and @AirQuick : which option would you prefer?

@AirQuick
Copy link
Member

I'm fine with 1.
If we go for 1 and if AppVeyor is annoyingly slow, I think we may retract 9.6.0-7 (or -10) only from AppVeyor.

@tofi86
Copy link
Author

tofi86 commented Jan 15, 2017

Okay, going to implement #1

@tofi86
Copy link
Author

tofi86 commented Jan 15, 2017

Done. Waiting for CI (timing) results…

@AirQuick
Copy link
Member

Funny they went down 😮 https://appveyor.statuspage.io/

@tofi86
Copy link
Author

tofi86 commented Jan 15, 2017

hahaha, hope it wasn't me 😂

@AirQuick
Copy link
Member

@cirulls , can you cancel the job?
Maybe it is straying like this.

@AirQuick
Copy link
Member

The oXygen forum thread is now visible. Thank you for pinging them.

@cirulls
Copy link
Member

cirulls commented Jan 16, 2017

I cancelled the hanging job, restarted a new build and tests are passing for all three versions of Saxon: https://ci.appveyor.com/project/xspec/xspec/build/1.0.56 It took about 5 minutes to run the test suite on AppVeyor. I guess it is fine but if it reaches 10 minutes in the future we should probably look at running tests in parallel on AppVeyor.

@cirulls cirulls merged commit f2db6e4 into xspec:master Jan 16, 2017
@cirulls cirulls added this to the v0.6.0 milestone Jan 16, 2017
@tofi86 tofi86 deleted the patch-1 branch January 16, 2017 08:42
@cirulls
Copy link
Member

cirulls commented Jan 16, 2017

By the way, is Syncro Soft aware of this repository? Will they sooner or later consider updating XSpec in oXygen based on this repository?

Alex Jitianu from Oxygen replied @tofi86's email:

Oxygen ships a quite old version of XSpec so we will definitely test the new version. If all goes well it will be present in Oxygen 19.0.

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

Successfully merging this pull request may close these issues.

None yet

3 participants