New PRs From Reactific

55 views
Skip to first unread message

re...@reactific.com

unread,
Dec 22, 2014, 8:50:42 PM12/22/14
to reacti...@googlegroups.com
Stéphane,

I have created six PRs for each of my commits, as detailed below. However I recommend that you consider 231, 232, 233 as a single patch as they are interrelated. You should also assess the impact to users of reactivemongo as it removes the ability to supply RM with an external ActorSystem and instead always creates on for RM. If that "feature" can't be done away with, there may be other things we can do, but I'd need to discuss them with you. The goal of those patches is to make it possible to have multiple MongoDrivers that keep their business separate and other enhancements to make startup/shutdown much more reliable.  The other patches, however, are straight up improvements that you will probably want. Here's the list:

  • Reliable Shutdown, Non-opinionated Logging, Self-Contained Actor System - #231
  • Make Monitor, Connection and Supervisor names unique - #232
  • Permit MongoDriver to be configured - #233
  • Reduce Warnings And Deprecations - #234
  • Upgrade to Scala 2.11.4 - #235
  • Fix Equality And Other Issues - #236
Let me know how you would like to proceed.

Best,

Reid.

On Monday, December 22, 2014 8:12:29 PM UTC-5, Stéphane Godbillon wrote:
I've had a glance to your commits and your changes seem to be quite interesting! :)  It would be great to issue a PR for them.  Then I will review it closely.  If you're willing to do so, please do not squash – it's better to keep the commits meaningful.

Cheers,

re...@reactific.com

unread,
Dec 23, 2014, 3:06:49 PM12/23/14
to reacti...@googlegroups.com
Stéphane,

Please ignore all those Issues that I opened (231-236) because I've now closed them all :) I thought I could attach the PRs to them later, but apparently GitHub doesn't allow that. 

Since I had to go back and make PRs, I decided to do it in a way that was friendly to the project. Some of the patches are, perhaps, premature and some make API decisions you might not want. In any event, there is only four things to consider now:
  1. Fix Equality And Other Issues, #238. This is a straight up bug fix with a test case for Equality. It actually is about 6 bug fixes including an unimplemented buffer.write method. The idea is to make BSON objects reflective through serialization/deserialization. Without this patch any BSONDocument that contains a BSONObjectID, BSONBinary or BSONDBPointer will fail equality after serialization/deserialization. The test suite tests these individually and in conjunction with a BSONDocument.  You likely really want this patch. 
  2. Modernize SBT Dependency Versions , #239. This patch is optional but I found it sped things up as it uses sbt 0.13.7 whose dependency resolution, especially for locally published things, is much faster -- big win for dev productivity. It also updates the underlying sbt plugins as well. 
  3. Minimize warnings, #240. This gets ride of the 100s of compiler warnings due to compiling with 2.11.4. Mostly these are deprecated macro features, things that need to be changed before 2.12 (next year sometime). While this is premature, I made this commit in my fork so I could see what was going on. There were so many warnings I couldn't find the errors. The big issue with this patch is that it no longer supports cross-compilation or Scala 2.10 as it uses Scala 2.11 only features. I'm not sure how long you plan to continue to support 2.10 but I stopped using it nearly a year ago when 2.11.0-M8 came out. In any event, you don't want to do this patch until you can drop support for 2.10. Maybe the ReactiveMongo community is ready now and we should poll this group for Scala version sensitivity?
  4. Improve Shutdown Reliability, #241. This is the main patch that started me working on ReactiveMongo. I think it is a significant improvement but it also comes with a breaking API change: you can no longer initialize the MongoDriver with an Akka ActorSystem. That's because MongoDriver, with this patch, always creates its own ActorSystem and always shuts it down on MongoDriver.close. It also names the actors uniquely, provides a Supervisor actor for the MongoDriver, and fixes some bugs in reliability of closing connections and the driver itself. 
Hope this helps. If you have questions, please ask here or privately. I'm happy to help keep this project moving along. 

Best,

Reid.

re...@reactific.com

unread,
Jan 12, 2015, 4:58:48 PM1/12/15
to reacti...@googlegroups.com
Any more feedback on these changes?

I'm nearing release of my project and trying to decide if I need to release a fork of RM or whether these changes will get included in the main project. It doesn't matter to me which way it will go, I just need to know so I can proceed. However, I think these changes are beneficial for RM and its users. 

Please let me know, one way or the other.

Thanks,

Reid.
Reply all
Reply to author
Forward
0 new messages