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.

Sign up for GitHub

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

Jump to bottom

Add a fallback to git diff command to get diff of a GitHub pull request #1697

Merged
merged 13 commits into from
Apr 13, 2024
Merged

Add a fallback to git diff command to get diff of a GitHub pull request #1697

merged 13 commits into from
Apr 13, 2024

Conversation

massongit
Copy link
Contributor

@massongit massongit commented Mar 31, 2024 •
edited

  • Updated Unreleased section in CHANGELOG or it's not notable changes.

Fix #1696

Based on #1696 (comment), I add a fallback to git diff command to get diff of a GitHub pull request.

@massongit massongit changed the title Use git diff command to get diff of a GitHub pull request [WIP] Use git diff command to get diff of a GitHub pull request Mar 31, 2024
@massongit massongit marked this pull request as draft April 1, 2024 00:17
@massongit massongit changed the title [WIP] Use git diff command to get diff of a GitHub pull request Use git diff command to get diff of a GitHub pull request Apr 1, 2024
@atiqueakhtar
Copy link

Hi @massongit ,

Thank you for investigating the fix for this issue. Are we on track to resolve the check failure (reviewdog / reviewdog (github-check) (pull_request)) soon? We're eager to know if this can be addressed soon, as the resolution is critical for our pending PRs for which reviewdog is failing.

Appreciate your efforts on this.

@massongit
Copy link
Contributor Author

I think it will take several days to weeks to resolve this issue.

@massongit
Copy link
Contributor Author

CI reviewdog / reviewdog (github-check) (pull_request) seems to fail if a pull request is created from a forked repository.
Therefore, I leave it as is.

@massongit massongit marked this pull request as ready for review April 1, 2024 16:20
@radiantshaw
Copy link

Therefore, I leave it as is.

@massongit Does that mean this PR will not be merged anytime soon?

@massongit
Copy link
Contributor Author

massongit commented Apr 5, 2024 •
edited

@radiantshaw I think that it will be merged soon if the review passes.
This is because all CIs other than CI reviewdog / reviewdog (github-check) (pull_request) are passed.

@dprotaso dprotaso mentioned this pull request Apr 5, 2024
Merged
@massongit
Copy link
Contributor Author

massongit commented Apr 8, 2024 •
edited

@shogo82148 @haya14busa Please review this PR.

haya14busa
haya14busa requested changes Apr 8, 2024
View reviewed changes
Copy link
Member

@haya14busa haya14busa left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution.

It's a bit tough problem and I'm not sure what's the best way to fix it.

doghouse/server/github_checker.go Outdated Show resolved Hide resolved
service/github/github.go Outdated Show resolved Hide resolved
service/github/github.go Show resolved Hide resolved
.github/workflows/reviewdog.yml Outdated Show resolved Hide resolved
@haya14busa
Copy link
Member

CI reviewdog / reviewdog (github-check) (pull_request) seems to fail if a pull request is created from a forked repository.
Therefore, I leave it as is.

btw, it's actually unexpected. reviewdog should not fail from a forked repository too.
I guess there are some bug about exit code handling 🤔

@massongit massongit changed the title Use git diff command to get diff of a GitHub pull request Add fallback to git diff command to get diff of a GitHub pull request Apr 8, 2024
@massongit massongit changed the title Add fallback to git diff command to get diff of a GitHub pull request Add a fallback to git diff command to get diff of a GitHub pull request Apr 8, 2024
@massongit
Copy link
Contributor Author

#1697 (comment)

btw, it's actually unexpected. reviewdog should not fail from a forked repository too.
I guess there are some bug about exit code handling 🤔

I fixed it: #1707
Therefore, please also review it.

@atiqueakhtar
Copy link

Hi @massongit ,

Thank you for the quick updates in response to the feedback!

Could the reviewers @haya14busa and @shogo82148 please take a look at the recent changes when you have a moment?

Thank you for your efforts on this!

haya14busa
haya14busa approved these changes Apr 13, 2024
View reviewed changes
Copy link
Member

@haya14busa haya14busa left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution 👍

@haya14busa haya14busa merged commit efeb8b0 into reviewdog:master Apr 13, 2024
15 checks passed
@massongit massongit deleted the use_git_diff branch April 14, 2024 07:59
@narutox900 narutox900 mentioned this pull request Apr 15, 2024
Closed
tricknotes added a commit to tricknotes/action-brakeman that referenced this pull request Apr 17, 2024
@tricknotes
Bump reviewdog version to v0.17.3
000f7ca 
Notable change:
- reviewdog/reviewdog#1697

All changes:
- https://github.com/reviewdog/reviewdog/releases/tag/v0.17.2
- https://github.com/reviewdog/reviewdog/releases/tag/v0.17.3
- reviewdog/reviewdog@v0.17.1...v0.17.3
@tricknotes tricknotes mentioned this pull request Apr 17, 2024
Merged
@massongit massongit mentioned this pull request Apr 25, 2024
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@haya14busa haya14busa haya14busa approved these changes

@shogo82148 shogo82148 Awaiting requested review from shogo82148

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

GitHub Pull Request diff API responds with 406 — diff too large
4 participants
@massongit @atiqueakhtar @radiantshaw @haya14busa