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

Various bits of build file Gardening #1950

Merged
merged 16 commits into from
Jun 17, 2018
Merged

Various bits of build file Gardening #1950

merged 16 commits into from
Jun 17, 2018

Conversation

farmdawgnation
Copy link
Member

This PR mostly consists of version bumps in our dependencies in anticipation of the Lift 3.3.0-RC1 release in a few weeks. There are a few minor, unrelated changes as well.

lazy val mongo_java_driver = "org.mongodb" % "mongodb-driver" % "3.6.3"
lazy val mongo_java_driver_async = "org.mongodb" % "mongodb-driver-async" % "3.6.3"
lazy val joda_time = "joda-time" % "joda-time" % "2.10"
lazy val joda_convert = "org.joda" % "joda-convert" % "2.0.2"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a major version bump. Any concerns around that?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, well the only thing this touches is the json-ext package, so none major from the framework's point of view. I guess in theory any of these could break transitive binary/API compatibility, but given how poorly developers tend to enforce semver it may be impossible to preserve that with any dependency bump. :/

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be clear, certainly worth investigating their change log when I get a chance though.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looked through the changes report and there are no breaking changes that I can see mentioned. It looks like they bumped the major version number when they re-worked the build to support Java 9:

http://www.joda.org/joda-convert/changes-report.html

I'm actually going to go ahead and bump this to 2.1, as well, since they just released that.

@farmdawgnation
Copy link
Member Author

Looks like the htmlparser bump broke template processing. I'm going to revert that change.

Looks like there were some subtle behavior changes we'll need to account
for here.
@farmdawgnation farmdawgnation merged commit 62e04f6 into master Jun 17, 2018
@farmdawgnation farmdawgnation deleted the minor-bumps branch June 17, 2018 00:52
@@ -47,7 +47,7 @@ object Dependencies {
lazy val squeryl = "org.squeryl" % "squeryl" % "0.9.5-7" cross CVMappingAll
lazy val slf4j_api = "org.slf4j" % "slf4j-api" % slf4jVersion
lazy val scala_xml = "org.scala-lang.modules" %% "scala-xml" % "1.0.6"
lazy val rhino = "org.mozilla" % "rhino" % "1.7.7.1"
lazy val rhino = "org.mozilla" % "rhino" % "1.7.10"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this was a million years ago but any idea for the reason behind it?

No biggie -- I'm just currently in the flail-in-the-dark phase of bumping us to Lift 3.4.3 and am going error by eror 😅 .

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think most of this PR was just getting us up to date with regard to our outdated dependencies.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

iiiiinterestingly enough:

we upgraded to lift 3.3 and our tests started failing. adding exclude("org.mozilla", "rhino"), to our lift dependencies got 'em to pass O___o.

not sure if we're just postponing the inevitable but i found it noteworthy.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's what got me asking about why this updated dependency -- we were only successful after we reverted it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's possible that you have other dependencies that rely on there being an older version of rhino present and Lift bringing in a newer version transitively causes issues for you. I'd recommend taking a look at what other dependencies might ask for Rhino (or look for any direct dependencies on it in your project) to try and determine whether or not that's what is happening. If you use Rhino directly, there's a good chance you'll have code to update.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is direct usage in that codebase fwiw 😉

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's possible that you have other dependencies that rely on there being an older version of rhino present and Lift bringing in a newer version transitively causes issues for you.

ooh yeah. I think that's why it the exclude worked, yah?

I suppose I expected something Rhino-y in this PR. That is, instead of just bumping a version, maybe something like rhino.oldFunction -> rhino.newFunction. On the other hand, version bumps to stay current -- with no code changes -- are also legit.

There is direct usage in that codebase fwiw 😉

'preciate it :D Suppose I might have to Actually Dig In:tm: & learn myself some Clojure 😎.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caveat -- I say "Clojure" 'cause I think that's how we're using it. Prolly shoulda read the README first 😅 .

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.

5 participants