Skip to content
This repository has been archived by the owner on Nov 28, 2024. It is now read-only.

Add option to no-op if target branch is up-to-date. #83

Closed
wants to merge 1 commit into from
Closed

Add option to no-op if target branch is up-to-date. #83

wants to merge 1 commit into from

Conversation

tomprince
Copy link

Fixes #82.

@tomprince tomprince mentioned this pull request Oct 8, 2019
Closed
jd
jd reviewed Oct 8, 2019
View reviewed changes
Copy link
Member

@jd jd left a comment

Choose a reason for hiding this comment

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

I'm struggling with 2 things:

  1. when is that useful?
  2. the name of the option sounds confusing

@tomprince
Copy link
Author 

1. when is that useful?

I've got some code in automation to port changes between two repositories, and then create a pull-request with the result. There sometimes isn't any changes that would result, in which case the code creates a branch that matches the upstream branch.

git-pull-request does all the work necessary to make a pull-request to bring the target branch up-to-date with the current branch, except in the case where it is already up-to-date (in which case it fails). I'd like to have an option where it will handle that case, but still error out if there are other problems creating the pull request.

2. the name of the option sounds confusing

I don't have strong feelings on the name, and agree that it isn't the clearest. I didn't have any better ideas, though.

@jd
Copy link
Member

jd commented Oct 16, 2019

It sounds like you can already do this by catching the exit code 50 in your calling program/script, no?

jd
jd suggested changes Nov 7, 2019
View reviewed changes
git_pull_request/__init__.py
_format_github_exception("create pull request", e)
)
return 50
if allow_noop and any((
Copy link
Member

Choose a reason for hiding this comment

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

I think it's fine to fail with that. I'd just not add a new option for that.

sileht
sileht suggested changes Dec 1, 2019
View reviewed changes
Copy link
Member

@sileht sileht left a comment

Choose a reason for hiding this comment

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

.

@jd
Copy link
Member

jd commented Jun 24, 2020

No update in a while, closing.

@jd jd closed this Jun 24, 2020
@escapewindow escapewindow mentioned this pull request Jul 11, 2022
Merged
escapewindow added a commit to mozilla-l10n/android-l10n-tooling that referenced this pull request Jul 11, 2022
@escapewindow
fix failure on empty pull requests (#45)
5d59758 
Mergifyio/git-pull-request#83 was rejected. Let's work around the issue
on our side.

Fixes #13.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@sileht sileht sileht requested changes

@jd jd jd requested changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add an option to succeed if there are no changes.
3 participants
@tomprince @jd @sileht