You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
while working on #7824 I noticed that it is possible, if main is empty, to:
create branch a from main
create branch b from main
make an empty commit to a
merge a to b without --allow-empty
It doesn't seem very important but for consistency I believe it should require --allow-empty or --force. It does require these flags if main is not empty.
If I understand correctly, it can be solved by adding a check in graveler\commited\manager.go:Merge if source metaRangeId == destination metaRangeId and require the flags in this case.
I can create a PR if you think it should be changed.
The text was updated successfully, but these errors were encountered:
Good point. I checked and git does allow such a merge (without additional flags), whether main is empty or not. lakefs already doesn't follow git in that it requires --force when main is not empty.
So the question is whether it's better to be consistent with git in the case that main is empty, or being consistent with lakefs's behavior when it's not.
If there's no simple answer that's clearly correct, do what Git does.
The reasoning is that Git had been around for many more years, has more users, and if a user has any preconception about what should happen -it's probably been influenced by Git.
Given that it's not clearly wrong, let's do what Git does here.
while working on #7824 I noticed that it is possible, if main is empty, to:
create branch a from main
create branch b from main
make an empty commit to a
merge a to b without --allow-empty
It doesn't seem very important but for consistency I believe it should require --allow-empty or --force. It does require these flags if main is not empty.
If I understand correctly, it can be solved by adding a check in graveler\commited\manager.go:Merge if source metaRangeId == destination metaRangeId and require the flags in this case.
I can create a PR if you think it should be changed.
The text was updated successfully, but these errors were encountered: