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

chore: remove comment in PR title check pipeline #2837

Merged
merged 3 commits into from
Apr 7, 2024

Conversation

HerrCai0907
Copy link
Member

No description provided.

@CountBleck
Copy link
Member

@HerrCai0907 Is a comment on the PR even necessary? Maintainers can just edit the PR title as needed, so we can avoid the extra dependency.

@HerrCai0907
Copy link
Member Author

@HerrCai0907 Is a comment on the PR even necessary? Maintainers can just edit the PR title as needed, so we can avoid the extra dependency.

I think it can help developer easy to find what happened and why my PR is rejected.
If you think we should keep less dependence even in CI pipeline, it is fine to remove related logic

@HerrCai0907
Copy link
Member Author

I will remove related logic since I cannot find a solution to write comments now. It is weird because in last PR, it can work😕.

@HerrCai0907 HerrCai0907 changed the title chore: add pull-requests write access to pr title action chore: remove comment in PR title check pipeline Apr 7, 2024
@CountBleck
Copy link
Member

Oh, that's strange...

@CountBleck
Copy link
Member

CountBleck commented Apr 7, 2024

Note that, if the PR comes from a fork, it will have only read permission despite the permissions given in the action for the pull_request event. In this case, you may use the pull_request_target event. With this event, permissions can be given without issue (the difference is that it will execute the action from the target branch and not from the origin PR).

In #2836, the PR was from a branch on this repo, but this PR is from a branch on your fork.

@HerrCai0907
Copy link
Member Author

Let's remove comments simply and avoid permission problem. WDYT?

@CountBleck
Copy link
Member

That's fine by me. I can't speak for anyone else though.

@HerrCai0907 HerrCai0907 merged commit 721236d into AssemblyScript:main Apr 7, 2024
15 checks passed
@HerrCai0907 HerrCai0907 deleted the chore/ci branch April 7, 2024 04:34
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.

2 participants