Skip config dependant specs

[JonRowe]

@pirj further work to allow independent merging of PRs

[pirj]

Looks good, LGTM.

But still, do we really need this? I'm not sure I understand why. That subsequent PR removes in_fully_monkey_patched_rspec_environment altogether.

For an all-green merge of those three PRs? They were green against each other, and they will still be red against main after merging.

[JonRowe]

I'm trying to get it so that each PR can be merged individually, this is always the preference unless it is impossible / extremely difficult, which these changes should not be. If we can avoid it we don't merge broken code. This is because it is a lot easier as a maintainer to make and review changes with green builds.

They only need to be green against 4-0-dev, these changes are breaking and cannot be merged to main.

[JonRowe]

Merging, build failure is due to minitest on Ruby 3.1 (the new head)

[pirj]

They only need to be green against 4-0-dev, these changes are breaking and cannot be merged to main.

The only difference between 4-0-dev and main is Ruby < 2.3 support (link will rot once #2803 is merged).

Scraping out monkey patching is a different story. Those changes are what makes repos incompatible with main/4-0-dev.

[JonRowe]

Scraping out monkey patching is a different story. Those changes are what makes repos incompatible with main/4-0-dev.

We are talking about the monkey patching removal, those PRs only need to be green against 4-0-dev as they contain breaking changes that cannot be merged to main until 4.0.0 is released. This PR allowed those PRs to go green against 4-0-dev which was my aim.

[JonRowe]

I apologise, this might be less confusing if we had named main 3-11-dev and simply kept changing the default branch each time we released a new version... We've never really had "one" root branch on RSpec, always a "dev" branch per major version and a "maintenance" branch per major/minor pair, with old dev branches being merged into by new dev branches on release.

By preference we always want to have a green PR for merging into a dev branch if possible, pinning between the three is the last resort when its not otherwise possible, because it's a pain to have to review and merge three/four PRs simultaneously.

[pirj]

Right. But this PR has nothing to do with main/3-11-dev, right?

Those three PRs, #2803, rspec/rspec-expectations#1245, rspec/rspec-mocks#1365 were meant to be merged together. They have inter-dependent changes, specifically:

• rspec-core removed disable_monkey_patching! that rspec-expectations' and rspec-mocks' specs used to use
• rspec-mocks and rspec-expectations removed their syntax= setting that rspec-core runtime used to use

Their builds pointing to each other's branches (using maintenance-branch trick) were green.
And if they were merged at the same time, 4-0-dev builds would be green.

This the point I suggested to merge rspec-core first, even though without maintenance-branch (or Gemfile's branch setting for rspec-expectations and rspec-mocks) pointing to remove-monkey-patching branch the build would be red.

I still don't quite understand why we need to make those intermediate builds green.
Why is it required for any of them to be backwards-compatible with anything, if those three PRs are essentially one big PR, it just happens to be spread over our repos.

And at this moment I'm puzzled how to rebase the following:

I'm going this way:

But now it implies that after merging those three PRs in question I'll have to send a cleanup PR to remove this skip unless check, is that correct?

Or is it ok to remove this line as well right away?

[JonRowe]

Right. But this PR has nothing to do with main/3-11-dev, right?

Correct

Those three PRs, #2803, rspec/rspec-expectations#1245, rspec/rspec-mocks#1365 were meant to be merged together. They have inter-dependent changes, specifically:

rspec-core removed disable_monkey_patching! that rspec-expectations' and rspec-mocks' specs used to use
rspec-mocks and rspec-expectations removed their syntax= setting that rspec-core runtime used to use
Their builds pointing to each other's branches (using maintenance-branch trick) were green.
And if they were merged at the same time, 4-0-dev builds would be green.

This the point I suggested to merge rspec-core first, even though without maintenance-branch (or Gemfile's branch setting for rspec-expectations and rspec-mocks) pointing to remove-monkey-patching branch the build would be red.

I still don't quite understand why we need to make those intermediate builds green.

I have been able to merge one PR because its ready, I am reviewing another which needs changes, and the final one needs those two PRS merged. The first two PRs were not dependant at all on any other PR after this was merged. This was the point. To remove the interedependency.

Why is it required for any of them to be backwards-compatible with anything, if those three PRs are essentially one big PR, it just happens to be spread over our repos.

They are not backwards compatible, its just about reducing the size and scope of the PRs. Massive commits / PRs are harder to reason with and understand. By making these small changes (in this PR) it stops it becoming one monster PR (which is undesirable) and makes it possible to review and merge smaller PRs, which are always preferred as they are easier to review and understand.

And at this moment I'm puzzled how to rebase the following:

The correct rebase is to remove the changes this PR makes. The rspec-core PR will be last, and will remain red until the other PRs are done, at which point its build can be kicked over and it reviewed and merged.

[pirj]

To remove the interedependency.

Still didn't work 😄
https://github.com/rspec/rspec-core/pull/2803/checks?check_run_id=1611669725

smaller PRs, which are always preferred as they are easier to review and understand

Completely agree in general.

In this specific case you mean the purpose of this is to be able to review these three PRs independently from each other?
Got it.

[pirj]

@JonRowe Quick approve? rspec/rspec-expectations#1258

[JonRowe]

Still didn't work 😄

It did work, it wasn't supposed to make rspec-core independent, it was supposed to make rspec-mocks and rspec-expectations independent. Mocks was merged, and expectations is green.

In this specific case you mean the purpose of this is to be able to review these three PRs independently from each other?
Got it.

This specific PR and its predecessor, where meant as interim steps specifically to allow the rspec-mocks and rspec-expectations to be reviewed and merged using 4-0-dev on rspec-core.

[JonRowe]

Ah I see what you mean, a cucumber scenario that wasn't picked up in CI