-
Notifications
You must be signed in to change notification settings - Fork 756
external_merge: pass the repo_path to the merge tool #7292
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
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
|
@jj-vcs/maintainers can someone please have a look? I like it and the implementation seems quite reasonable to me |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SGTM. I suspect we might want to rename the variable to $path instead for consistency withjj fix.
541151a to
e9410e1
Compare
This follows Git's merge driver interface which makes this information available using the `%P` variable. It is useful for merge drivers to adapt their behaviour based on the type of file supplied.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks.
|
So what should have happened here is that you should have been added to contributors so you can merge your own PR. I'll merge it for now, but some maintainer should give you the role for future PRs. |
|
@algmyr fascinating! I'm very interested in FOSS governance so I'm curious how this invitation to the contributors team is supposed to happen. Is there some automation behind it, perhaps? Or is it the responsibility of a particular person/team? |
|
There is no automation at the moment, and I think only the team of Maintainers have the power to give the membership. Sadly forgetting to add people happens more often than it should. I hope #7606 should help with the not forgetting part once we get that in. @jj-vcs/maintainers Can anyone add wetneb to contributors? |
This follows Git's merge driver interface which makes this information available using the
%Pvariable. It is useful for merge drivers to adapt their behaviour based on the type of file supplied.The motivation for this is that Mergiraf uses this information to determine the format of the file and parse its revisions accordingly. When the file format is determined by the file extension only, there is no need for this extra information as the file names of the base, left and right revisions will still have this extension (such as
base_main.rs,left_main.rs,right_main.rs). But for files where the format is recognized by matching the entire file name, this will not work:base_Makefileisn't recognized on its own. However, when-p src/Makefileis provided, the tool is able to pick the right parsers.Checklist
If applicable:
CHANGELOG.mdREADME.md,docs/,demos/)cli/src/config-schema.json)