Revert previously reverted changes + add fix

[svuckovicTT]

• Several changes were reverted in Reverted broken commits + updated docs #454 - the change has been "unreverted" + the fix for the broken test has been added

[tapspatel]

thanks so much @svuckovicTT !!! lgtm

[svuckovicTT]

I'd prefer to merge this WITHOUT squashing. Since there's multiple unrelated commits in this change, it'll be easier for someone to revert/cherry-pick a specific commit down the road, if there's a need. Lmk if you have any objections to that.

[nsmithtt]

Thanks @svuckovicTT

[nsmithtt]

I'd prefer to merge this WITHOUT squashing. Since there's multiple unrelated commits in this change, it'll be easier for someone to revert/cherry-pick a specific commit down the road, if there's a need. Lmk if you have any objections to that.

Perhaps we can do them as separate PRs then? The reason for the hard enforcement of squash is that it drastically simplifies the git history and CI pipeline history, on main. It enables someone to checkout any commit on main and build and run and pass all unit tests. If we don't' enforce squashing then there are gaps, this is bad for 2 reasons:

• Someone could checkout some commit on main that's in an undefined state, it's just in the middle of some larger change, may or might not pass unit tests, who knows.
• Unable to generically use git bisect for regression testing. If every commit is expected to work and tested to some degree of confidence, then git bisect can freely walk the tree and have a fully automated bisect.

There's definitely tradeoffs with this approach, but having this invariant is extremely powerful and usually outweighs the downsides. I'd also argue that this method makes it easier to revert or cherry-pick, there are very few cases in my mind where you'd want to only cherry-pick half of a change, if a change could have been broken into pieces it probably should have been separate PRs to begin with.

[svuckovicTT]

I'd prefer to merge this WITHOUT squashing. Since there's multiple unrelated commits in this change, it'll be easier for someone to revert/cherry-pick a specific commit down the road, if there's a need. Lmk if you have any objections to that.

Perhaps we can do them as separate PRs then? The reason for the hard enforcement of squash is that it drastically simplifies the git history and CI pipeline history, on main. It enables someone to checkout any commit on main and build and run and pass all unit tests. If we don't' enforce squashing then there are gaps, this is bad for 2 reasons:

• Someone could checkout some commit on main that's in an undefined state, it's just in the middle of some larger change, may or might not pass unit tests, who knows.
• Unable to generically use git bisect for regression testing. If every commit is expected to work and tested to some degree of confidence, then git bisect can freely walk the tree and have a fully automated bisect.

There's definitely tradeoffs with this approach, but having this invariant is extremely powerful and usually outweighs the downsides. I'd also argue that this method makes it easier to revert or cherry-pick, there are very few cases in my mind where you'd want to only cherry-pick half of a change, if a change could have been broken into pieces it probably should have been separate PRs to begin with.

Ah, I didn't realize that squashing is enforced - I see your points though, and agree that it's better to squash.

Luckily, the PR will remain, and the commits in the PR itself will remain separate so if anyone needs to cherry pick anything, they can.