General thoughts on merge-process

[infabo]

On a quick hop through the 1.9.2.4-branch tree I found some orphan files like connect package xmls from Magento 1.9.1.0.

https://github.com/OpenMage/magento-lts/tree/1.9.2.4/var/package

I have not investigated further (like doing a real diff on a vanilla 1.9.2.4), but things like this always made me suspiciously looking at "public magento mirrors".
Of course, nobody will ever do a mage upgrade-all when using this repo.
But what I wanted to pick up basically: is it the only "merge-fail"?

This is not a rant, I rather want to point out a serious problem.

[drobinson]

Hi @infabo - thanks for pointing this out. Seems it was missed while importing the 1.9.1.1 sources. More recent version imports use the process outlined here: #112

which would explain the orphan files still sticking around until now. We'll take this as a bug and remove them asap.

In general merges require at least 2 maintainers' approval. Please let me know if you find anything else like this and we can fix it - but you're right, it's a public Magento mirror that only has a few people behind it (rather than an entire org) so there may be things like this. The beauty is we can hopefully fix them quickly once pointed out since this repo more-or-less belongs to the community.

[infabo]

Github PR reviews are great. I don't know if that already applied for the merge back then.

[colinmollenhour]

The new reviews feature is definitely a huge improvement although I think if we had a few more members it would be good to require three reviewers instead of one.

In this particular case I don't see a few spare xml files as being a serious problem and in general I think we are seeing good bug fixes that make LTS worth using over official CE. Perhaps we should add a "Use at your own risk!" statement to the README? :)

[drobinson]

@colinmollenhour I think we can probably move ahead with increasing the min #approvals to at least 3. The 2 person approval came from when me, @LeeSaferite, and @Flyingmana were the only active maintainers. Recently it looks like we have many more than that (yay!) so let's go ahead and bump it.