Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge via intermediate branch #72

Merged
merged 3 commits into from
Nov 20, 2023
Merged

Merge via intermediate branch #72

merged 3 commits into from
Nov 20, 2023

Conversation

pastey
Copy link
Contributor

@pastey pastey commented Nov 17, 2023

Интерактивные вопросы при мерже разосшедшихся ревизий в сабрепах вдохновлялись HG.
В наших современных реалиях чаще всего не имеем права мержить (а главное пушить) напрямую в таргет ветку.
Завел переменную окружения, которую будет читать мерж драйвер, и если она есть, то перед мержем будет создаваться промежуточная мерж-ветка.
Например:

S7_MERGE_DRIVER_INTERMEDIATE_BRANCH=merge/release/pdfexpert-7.19.0/to/release/pdfexpert-next git merge origin/release/pdfexpert-7.19.0

Кроме собственно этой фичи в этом ПР два маленьких фикса. Будут описаны по дифу.

In our realities, we often cannot afford direct merge (and push) to the target branch because of Branch Protection Rules.
The new option instructs merge to create an intermediate branch on <local> before the actual merge with <remote>.
@@ -29,7 +29,8 @@ xcodebuild_cmd() {
# these headers are unwanted and may cause module import conflicts
unset USER_HEADER_SEARCH_PATHS
unset HEADER_SEARCH_PATHS
xcodebuild -target system7 -configuration Release DSTROOT="$HOME" clean install
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Xcode писал при сборке страшное сообщение warning: Refusing to delete /Users/<user>. Не хотелось бы чтобы у него когда-то это получилось 🙈

NSString *stdOutOutput = nil;
const int exitStatus = [self runGitCommand:[NSString stringWithFormat:@"merge-base %@ %@", possibleAncestor, possibleDescendant]
stdOutOutput:&stdOutOutput
const int exitStatus = [self runGitCommand:[NSString stringWithFormat:@"merge-base --is-ancestor %@ %@",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Воспользовался более современным ключем --is-ancestor. Меньше кода 🤷‍♂️
(на самом деле я пытался еще одну штуку улучшить, но там меня посетил облом, а это осталось)

Copy link
Contributor

@salpieiev salpieiev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Це перемога 🥳

Озвучу тут просто свои мысли, которые возникли при просмотре ПР. Environment Variable выглядит немного странно. У нас в s7 уже есть замена команде статус, вернее расширение. Возможно можно было сделать альтернативную команду и для мерджации, в параметрах которой будет промежуточная ветка. Но не уверен что это решение лучше чем то что тут сделано

@pastey pastey merged commit 0f7539b into main Nov 20, 2023
2 checks passed
@pastey pastey deleted the merge-via-intermediate-branch branch November 20, 2023 15:53
@pastey
Copy link
Contributor Author

pastey commented Nov 21, 2023

По поводу отдельной команды есть сомнения.

  1. люди любят GUI.
  2. в целом речь идет о merge driver-е, а он может вызываться в разных контекстах:
  • merge
  • git checkout -m
  • cherry-pick
  • apply patch
  • etc.
  • мы не сможем/не хотим на все это плодить команды в s7

У меня была еще мысль в процессе мержа каждый раз спрашивать,– "а не желаете ли промежуточную ветку?". Такой подход при большом мерже будет доставлять боль человеку, плюс автоматизировать это скриптами будет сложно, и мы все равно заведем environment variable.

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