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

Repository Dispatch: improve error message for fork PRs #97

Merged
merged 4 commits into from
Jun 3, 2022

Conversation

AgnesToulet
Copy link
Contributor

The RepositoryDispatch action is used by the Enterprise PR check to trigger a build on Enterprise to test OSS changes against Enterprise codebase. This is not working for forks as forks don't have access to the GH token to trigger the event and this is confusing for a lot of people that don't know if they should merge or not the PR. Adding this log should help reviewers understand why this error is happening and what is expected from them.

This PR also updates TS dependencies as before 4.6 and this change, it wouldn't allow code to be called before super().

@xlson adding you as a reviewer to check the log message, is it clear enough?
Example of how it appears in GH: https://github.com/grafana/grafana/runs/6709763832?check_suite_focus=true

Copy link

@xlson xlson left a comment

Choose a reason for hiding this comment

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

I think it's clear enough. I wonder if we should be adding a link to more information but I think we can do that later if we need to, better to get this merged now and see what effects it has.

Copy link
Contributor

@tolzhabayev tolzhabayev left a comment

Choose a reason for hiding this comment

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

Just some comments, if tested and it really works then LGTM!

repository-dispatch/index.ts Show resolved Hide resolved
repository-dispatch/index.ts Outdated Show resolved Hide resolved
@AgnesToulet AgnesToulet merged commit f834fde into grafana:main Jun 3, 2022
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