gruntwork-io / git-xargs

git-xargs is a command-line tool (CLI) for making updates across multiple Github repositories with a single command.

https://blog.gruntwork.io/introducing-git-xargs-an-open-source-tool-to-update-multiple-github-repos-753f9f3675ec

Apache License 2.0

933 stars 63 forks source link

Support for overriding base branch names #64

Closed nirnanaaa closed 2 years ago

nirnanaaa commented 2 years ago

Support overriding base branch name (Follow up to https://github.com/gruntwork-io/git-xargs/issues/22)

Description

Since our git workflow uses multiple branches of the same repository in some places, I thought it would make sense to support an optional argument for overriding the base branch name globally (might be an addition to #22).

Documentation

README
--base-branch-name=<xxx>

TODOs

Please ensure all of these TODOs are completed before asking for a review.

[x] Ensure the branch is named correctly with the issue number. e.g: feature/new-vpc-endpoints-955 or bug/missing-count-param-434.
[x] Update the docs.
[x] Keep the changes backward compatible where possible.
[ ] Run the pre-commit checks successfully.
[ ] Run the relevant tests successfully.

Related Issues

22

zackproser commented 2 years ago

Thanks so much for your contribution, @nirnanaaa! I'll take a look at this shortly.

zackproser commented 2 years ago

The implementation mostly looks good to me! I added some code necessary for allowing the parsing of the new --base-branch-name flag.

In local testing, I found one issue that I'll need to fix prior to us merging this in: namely the behavior where we key off an HTTP 422 status code from Github whenever a pull request fails to be opened.

Currently, we're using this return code to determine when someone has requested a draft pull request via the --draft flag on a repo that does not support drafts. However, Github's API also returns a 429 when the base branch target is invalid - meaning the user specifies a base branch that does not exist.

Before we can merge this, I'll need to add logic to determine which type of HTTP 429 error we received: a) the error relating to draft pull requests or b) the error relating to the base branch name provided by the user being invalid.

zackproser commented 2 years ago

The implementation mostly looks good to me! I added some code necessary for allowing the parsing of the new --base-branch-name flag.

In local testing, I found one issue that I'll need to fix prior to us merging this in: namely the behavior where we key off an HTTP 422 status code from Github whenever a pull request fails to be opened.

Currently, we're using this return code to determine when someone has requested a draft pull request via the --draft flag on a repo that does not support drafts. However, Github's API also returns a 429 when the base branch target is invalid - meaning the user specifies a base branch that does not exist.

Before we can merge this, I'll need to add logic to determine which type of HTTP 429 error we received: a) the error relating to draft pull requests or b) the error relating to the base branch name provided by the user being invalid.

Alright - did this in 1adbd58. Kicking off tests now and then will look for someone who's not me to review!

zackproser commented 2 years ago

Thanks for the review! Going to merge this in now.