Skip to content

Conversation

@johnhaddon
Copy link
Member

@johnhaddon johnhaddon commented Mar 2, 2023

In which John realises he has made a horrible mistake :

  1. After I merged MeshSplitter #1333, I realised it had been targeting main and not RB-10.4 as it should have been.
  2. So I merged the PR branch to RB-10.4 manually on the command line too.
  3. But I didn't notice that the PR branch was branched from main too, and therefore contained all the commits from main as well as the MeshSplitter code we wanted.
  4. So now RB-10.4 contains everything from main.
  5. Sorry. If there was an emoji for a muppet, I would put it here a thousand times.

I've "fixed" this in this PR with a commit that reverts everything from that merge. Then I've cherry-picked the MeshSplitter commits we do want, and done the usual commits to bump version and update Changes for a new release. Presumably, I will have to also revert the revert when next merging RB-10.4 to main.

In practice, very little had been done on main since we branched RB-10.4. The main things I see are removal of a 3Delight display driver (which doesn't work with recent 3Delight any more) and some code changes for Imath 3 (which shouldn't break ABI anyway). So, do verify this for yourself, but it seems possible that we could actually not revert the bad merge, if we were feeling in a positive sort of mood. But I want to provide all options I can, hence the revert commit - let me know if you think it best to keep it or remove it.

Daniel, I've tested the cherry-picked MeshSplit stuff in a Gaffer build and all seems well, but please do give it a good check yourself to be sure I've got all the right things.

That leaves the final 3 commits, which are the only bits that would be necessary if I wasn't a colossal plonker.

@danieldresser-ie
Copy link
Contributor

I'm not seeing anything here that's scary in the Imath changes, and if the 3delight driver isn't useful currently anyway, I don't really see any problem with just moving forward.

@murraystevenson
Copy link
Collaborator

I'd be inclined to agree with Daniel, maybe we just carry on rather than deal with any further complications merging forward after rolling back. Chalk it up to the inevitable price of the convenience of command line surgery...

This will allow us to use Cortex release packages in Gaffer 1.2.x.
@johnhaddon johnhaddon force-pushed the releasePrep-10.4.6.0 branch from 5a213c6 to f69a29a Compare March 3, 2023 09:33
@johnhaddon
Copy link
Member Author

OK, I've checked through all the "bonus" commits once more and do think they're harmless. So I've simplified this PR down to just the version/dependency numps. Will merge once CI gives the OK.

@johnhaddon johnhaddon merged commit 16942e8 into ImageEngine:RB-10.4 Mar 3, 2023
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.

3 participants