🚀 MONGOID-5572 Rspec use expectation syntax

[johnnyshields]

Fixes MONGOID-5572

Done using Transpec gem

[johnnyshields]

@jamis can you take some time to merge this one soon? It's going to be a lot of busy work for me to keep updating conflicts here.

[jamis]

Hey @johnnyshields,

This...is going to be a lot of work to review. Just scrolling through the diff is intimidating; even with most changes following a fairly ubiquitous template, it's going to take a significant chunk of time to review each change.

I don't disagree that keeping this PR up-to-date with master is "a lot of busywork", as you say. But this is a very big reason why I generally disagree with this kind of PR: globally touching a lot of files at once in order to enforce a new style is a lot of noise, with very little signal.

My preference is to make these changes incrementally, as we touch the files in question. It may mean that some files are never updated to the new style, but that's okay.

I really do appreciate the time and effort spent on producing this PR, and maintaining it til now. Part of the blame here is on us for not reviewing it sooner, and for not having better documentation surrounding our contribution policies. I really am sorry for that--we're juggling a lot of priorities internally and it can be hard to make time for the smaller things.

I'll make some time today to draft a change to our contribution policy, and pitch it to the team. I think we need to clarify some of our expectations around PRs, and how we review and prioritize them. Part of what I intend to propose is, especially for larger changes like this, we would prefer to have the Jira ticket first, with a note indicating your willingness to do the work. That way, in our weekly triage we can discuss whether the work is even something we're interested in, before a contributor sacrifices their time on it.

For now, with a lot of reluctance, I'm going to close this PR, and the associated ticket. Please know this is no reflection on you, or your work. We all appreciate your passion for the project and your willingness to contribute, but this particular issue is not one we're willing to review right now.

[johnnyshields]

@jamis I'm shocked. How is this possibly intimidating? All you need to do is:

1. Check only spec files are being modified ✅
2. Check all specs continue to pass ✅
3. Do a rough read of the diff ✅

Then you can merge it.

How about I break it into 10 smaller "bite-sized", "non-intimidating" PRs to help your team here?

[jamis]

Johnny, I understand you're disappointed, but please don't stoop to "how is this possibly..." arguments. When someone expresses a feeling, the empathetic and kind thing to do is to acknowledge that they feel a thing. Whether or not you would feel similarly in the same situation is immaterial.

As to why I feel this way: a rough read of the diff is absolutely not sufficient when accepting code from external contributors. (Or even internal contributors, for that matter.) Whether it touches only tests or not, if I accept a PR, I also accept responsibility for that code. Even if, for the sake of argument, the PR is "just" specs, it is still code that will be run on my personal machine, and the machines of my coworkers. Also, other contributers are likely to run the specs on their machines. If by my lack of diligence someone sneaks some superficially-innocuous code into the codebase, which then winds up being damaging to me, my coworkers, other contributors, and potentially MongoDB at large, I am at fault.

So, no, I will not "do a rough read of the diff" and call it sufficient. And between this PR and the other two related PRs, the amount of content was not trivial. Even if you break it into smaller "bit-sized" PRs, that doesn't change the scope of the review.

I stand by my statement that we prefer these sorts of changes to be done incrementally. There is absolutely no urgency here that would require us to upgrade to a new syntax in bulk. There is no security advisory, no broken code, no emergency in the community at large that this is trying to fix. The priority is, frankly, quite low.

So, again, I really am sorry, and I know this is disappointing and frustrating for you. I take responsibility for the poor communication that led to this, and I'll do better in the future.

[johnnyshields]

Jamis, my point here is that if you find this PR intimidating--easily readable and affecting only spec files--then I really fear for the velocity of this whole project. In the time you spent writing me your lengthy reply, you could have reviewed and merged it. And it's insulting and ridiculous to suggest that I as a long-time collaborator--150+ PRs merged--and a CTO/Founder of a reputable company with 170 employees, am going to be "sneaking in superficially-innocuous code."

I'm getting to the point where it will make sense for my company to fork this project and release a "Community Edition". It would be a shame, but closing this latest PR (one of many that your team has closed inexplicably) further demonstrates the wide philosophical gulf between Mongoid and its user community.

I apologize if I am coming across as frustrated. It's because I am.

[johnnyshields]

This PR is merged into Mongoid Ultra Edition 🚀

[johnnyshields]

@jamis please consider re-opening this.