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

Feature Request: Ignore Fail #14

Merged
merged 5 commits into from
Aug 16, 2020
Merged

Feature Request: Ignore Fail #14

merged 5 commits into from
Aug 16, 2020

Conversation

AsheKR
Copy link
Contributor

@AsheKR AsheKR commented Jun 29, 2020

If there is no additional commit, there will be an error, so I would like to add a function that can skip this error.
if you have any ideas or corrections, please leave a comment

@R0Wi
Copy link
Contributor

R0Wi commented Aug 16, 2020

For me it also happened that i received an error because there haven't been commits between upstream and my fork. Usually this error should be handled here:

https://github.com/TG908/fork-sync/blob/e15fc5e857fee00f9ba8b239fc3546e4612ed0c6/src/main.ts#L20-L22

but in my case the error message was something like

No commits between master and master

so the check didn't succeed because it expects a error message like 'No commits between ' + context.repo.owner + ':' + base + ' and ' + owner + ':' + head.

But what i also noticed is the http error code 422 saying "Unprocessable Entity". So it might be an option to edit the mentioned line of code like so:

if (!!error.errors && error.status == 422 && error.errors[0].message.startsWith('No commits between')) {
      console.log('No commits between ' + context.repo.owner + ':' + base + ' and ' + owner + ':' + head);
    }

This would be a bit more robust i think.

@tgymnich
Copy link
Owner

Good catch. Thank you!

@tgymnich tgymnich merged commit 24ad132 into tgymnich:master Aug 16, 2020
@AsheKR
Copy link
Contributor Author

AsheKR commented Aug 18, 2020

@R0Wi
I think it's a different issue from that one.
I think it would be good to open and apply a new issue. 👍

@R0Wi
Copy link
Contributor

R0Wi commented Aug 18, 2020

Agree on that. I'll open a new issue with a PR attached :-)

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