sul-dlss-deprecated / sdr-preservation-core

Robots for ingesting objects into SDR Preservation Core -- replaced by preservation_robots
Other
1 stars 0 forks source link

[WIP?] rebase develop on master so it can be merged cleanly to master #56

Closed jmartin-sul closed 7 years ago

jmartin-sul commented 7 years ago

here's an attempt at rebasing develop on master. took a little while, but it was pretty mechanical, and didn't really require any hard decisions, so hopefully it works? this was my procedure:

anyway, this branch and its PR are the result of that process. hopefully that's helpful in getting develop merged.

you'll notice that some of the commits in this PR don't really do anything. i assume that's a result of the above rebasing process. i'm happy to pull them out of the PR or clean them up, but i wanted to start out with this branch being the result of the above totally mechanical process.

coveralls commented 7 years ago

Coverage Status

Changes Unknown when pulling 4ff514f9f089f1fd7d39b563f854e2985ebf60b6 on develop_try-rebase-2017-06-26 into on master.

jmartin-sul commented 7 years ago

turns out that regenerating Gemfile.lock inadvertently bumped activesupport from 4.x to 5.x, and sinatra from 1.x to 2.x. those seem to be indirect dependencies, so maybe that's actually safe? anyway, in case it isn't, i pushed a commit that brings Gemfile.lock back to activesupport 4.x and sinatra 1.x. if it turns out those upgrades are safe, then we can just get rid of the most recent commit, and keep the major version upgrades of those dependencies. i don't know this codebase well enough to judge the safety of that one way or the other.

coveralls commented 7 years ago

Coverage Status

Changes Unknown when pulling a86a427b592538fe2faa181385ef8a69daf59f6f on develop_try-rebase-2017-06-26 into on master.

jmartin-sul commented 7 years ago

upon closer inspection, the commit history needs some squashing and rewording, but will let others weigh in on the overall reasonableness of this PR before doing anything further.

dazza-codes commented 7 years ago

I'm concerned about the process of the rebase vs. a merge, in particular the bit about "to deal with Gemfile.lock conflicts, i first resolved the Gemfile conflict, then just deleted Gemfile.lock, and did a bundle install to regenerate it ....". It's one way to get it done and I'll look closer at the details today. I think wiping out the Gemfile.lock entirely is a kind of "brute force" approach, but it seems to have worked in the sense that the specs are passing. Thanks for taking initiative on this approach, it's on my agenda for today to look into this and similar approaches. Only to ensure that nobody merges this while I'm looking into this and similar alternatives, I'll mark this PR as requiring changes until I can evaluate details and possible alternatives.

dazza-codes commented 7 years ago

I'd like to move this work to #57 but leaving this open until we decide.

dazza-codes commented 7 years ago

robot-controller (2.1.1) did not install for me on ruby 2.1.2 when I was doing some conflict resolution earlier (the SDR-PC systems are running ruby 2.1.x). Trying to update it on the develop branch gets:

Gem::InstallError: mustermann requires Ruby version >= 2.2.0.
An error occurred while installing mustermann (1.0.0), and Bundler cannot continue.
Make sure that `gem install mustermann -v '1.0.0'` succeeds before bundling.

In Gemfile:
  robot-controller was resolved to 2.1.1, which depends on
    resque was resolved to 1.27.4, which depends on
      sinatra was resolved to 2.0.0, which depends on
        mustermann
jmartin-sul commented 7 years ago

weird. the requirement for mustermann should've been removed with the commit i added on at the end (and mustermann doesn't appear to be added on in the current diff for this branch). and i was able to do a bundle install of this branch using ruby 2.1.10 just now (locally on my laptop).

also, if the SDR boxes are running ruby 2.1.x, we should probably add that to the travis build matrix? i was going to say travis didn't complain about the installation, but appears to only check against ruby 2.2.4 (in all the currently available branches, AFAICT).