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

Pre-commit.ci failure due to permission denied #4589

Closed
maki49 opened this issue Jul 6, 2024 · 5 comments
Closed

Pre-commit.ci failure due to permission denied #4589

maki49 opened this issue Jul 6, 2024 · 5 comments
Assignees
Labels
Compile & CICD & Docs & Dependencies Issues related to compiling ABACUS

Comments

@maki49
Copy link
Collaborator

maki49 commented Jul 6, 2024

Up to now I find it only happens in @PeizeLin 's PR (like #4558 and #4590), so I guess it is because his repository is forked from abacusmodeling rather than deepmodeling. This problem should not block the merge.

image

@mohanchen mohanchen added the Compile & CICD & Docs & Dependencies Issues related to compiling ABACUS label Jul 7, 2024
@caic99
Copy link
Member

caic99 commented Jul 8, 2024

Hi @maki49 ,
Running pre-commit.ci services requires access to that repo, so your guess is possibly correct. We've asked the user to fork the repo from the deepmodeling community. And since pre-commit is not an action required to be passed, it is not blocking the merging progress.

@caic99 caic99 removed their assignment Jul 8, 2024
@mohanchen
Copy link
Collaborator

mohanchen commented Jul 8, 2024

Besides the solution you just provided, there is still an issue remains. Apparently, from the point of code reviewer, any PRs that failed the pre-commit is labelled as 'failed PR', making the reviewer very confusing. What's your suggestion to deal with this situation? @caic99

@caic99
Copy link
Member

caic99 commented Jul 8, 2024

@mohanchen From the view of the person who decides whether to merge that PR, the PR status page shows:
i. squash and merge, which means the required actions are passed and there is at least 1 approving review, or
ii. merging is blocked, which the condition above is not satisfied.
I think these info is sufficient for them.

@caic99
Copy link
Member

caic99 commented Jul 8, 2024

@mohanchen Or, the reviewer can request the developer to submit a PR from a repo forked from deepmodeling repo, ensuring the correctness.

@mohanchen
Copy link
Collaborator

Thanks!

@maki49 maki49 closed this as completed Jul 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Compile & CICD & Docs & Dependencies Issues related to compiling ABACUS
Projects
None yet
Development

No branches or pull requests

3 participants