Fix broken commit history and dangerous project settings

[cthoyt]

Here's what happened:

1. added new object property has roost for issue #453 #672 was merged, even though CI was failing. Why was it failing: this branch was created before migrating RO to use the ODK RO migration to ODK 1.4 #492/Release with Zero content changes #681 and had not been updated since. This meant that 1) the diff was big and confusing and 2) it lacked lots of other quality improvements done since, including switching from DCE to DC Terms (Ensure provenance relations are used with ORCID or other appropriate IRIs #690/Replace all uses of DCE with DC and add violation check #692).
2. Revert previous committed changes made in PR #672 and add missing axioms for has roost #730 was created to revert the commit, but never actually applied. @wdduncan you noted in the PR that you applied the revert, but this never changed the main branch
3. More complicated changes were made and merged in Clean up files #725, while the main branch was still failing

This leaves us in a place where there are huge, spurious diffs mixed up in bad changes that break CI. It makes it hard to figure out what changes happened and why.

What I suggest we do immediately:

1. Reset the main branch back to a6d1754
2. Re-do Revert previous committed changes made in PR #672 and add missing axioms for has roost #730 (sorry @anitacaron)
3. Change the settings for this repository to make sure that PRs can not be merged if CI is not passing
4. Fix Change dc:creator to dcterms:contributor #731 (it includes tons of spurious diff and is still not passing CI)

[wdduncan]

Yes. I merged the PR w/o noticing that it was failing. That is my fault.

I'm not sure we will be able to fix the commit history.

[cthoyt]

@wdduncan it is technically possible and I know how to do it, but will require some trickery and getting everyone on board with the idea that we should try and keep the commit history clean and meaningful

[gouttegd]

We’re mixing three different problems here:

1. Fixing the state of the master branch, which is currently broken. No argument here, we should absolutely do that. And for that, all we have to do is to merge Change dc:creator to dcterms:contributor #731 in its current state. It has been made precisely to fix the mess left by added new object property has roost for issue #453 #672, and it does pass the CI checks – it does not need more “fixing“, the “spurious diff“ are there precisely because they correct the spurious diff introduced by added new object property has roost for issue #453 #672.

2. Preventing this kind of things from happening in the future. Again, no argument, that’s obviously the correct thing to do.

3. Fixing the history of the master branch, so that it looks like the problem never happened in the first place. This involves re-writing the history to make all the commits of added new object property has roost for issue #453 #672 and Clean up files #725 completely disappear. @diatomsRcool and @anitacaron will have to redo the corresponding work (hoping they have kept it in a local branch somewhere). And whoever has already updated their local copy of the master branch since last Friday will have to fix their own copy of the history before they can update it to the new, re-written public master branch.

Option 3) is more work, more pain, for (in my opinion) no justifiable reason. I am all in favour of a “clean, meaningful history“ (hell, I am the one on record saying that people who write meaningless commit messages should be hung, drawn, and quartered), but history re-writing should only ever happen in private branches. Once the mess reaches a public branch, we must own it instead of going to great lengths to pretend it didn’t happen.

That being said, I don’t have any kind of authority in RO, so if that’s really what we want, then go ahead. But if we do that, we must do it ASAP (like, right now!) – the more time passes, the more likely it is that other people update their local repository with the current master branch.

[matentzn]

I personally think we should protect the master branch now, merge the fix, and leave the git history in whatever state it is in; this is not really like a software project where a broken master can break downstream code (if it was, I would agree that fixing the git history would make some sense).

[cthoyt]

alright, I guess I am outvoted. let's just get this stuff fixed ASAP

[balhoff]

Anyone, please go ahead and merge the fixed PR.

[cthoyt]

This issue can be closed once branch protections are set up on this repo

[balhoff]

@cthoyt do you have permissions to do that? If so that would be great, or else I can do it later once I get to my office.

[matentzn]

I have updated it now, no more merging, not even by maintainers.

[gouttegd]

this is not really like a software project where a broken master can break downstream code (if it was, I would agree that fixing the git history would make some sense).

For the record: no, it would not make any more sense. Whether it is a software project or anything else, if what you care about is “not breaking anything downstream“, then what is important is always the current state of the master branch. Whether the history of the branch contains broken commits is of no consequence – unless some people downstream decide to fork the branch at an arbitrary commit rather than at the tip, but if they do that, frankly it’s up to them to deal with the possibility that the commit they have chosen to fork off may be broken.

[anitacaron]

All settings are set to prevent merging a branch when there's an issue.

[anitacaron]

I ran a new RO release and PR #722 was reverted. Need to redo it.

[anitacaron]

The problem was only partially solved. The only PR that would solve is the one that was never merged, PR #730.