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

Add GitHub Enterprise support #108

Merged
merged 7 commits into from
May 3, 2022
Merged

Add GitHub Enterprise support #108

merged 7 commits into from
May 3, 2022

Conversation

ZPascal
Copy link
Collaborator

@ZPascal ZPascal commented Feb 18, 2022

Hi @ad-m,

the corresponding draft PR is a follow-up of the last PR to include the GTE support inside the action. Could you please modify the settings of the repository to make it possible to execute the check also from a fork?

To-do:

  • Check the GitHub API URL variable from the GitHub context inside the GTE instance.

@ad-m
Copy link
Owner

ad-m commented Feb 18, 2022

Could you please modify the settings of the repository to make it possible to execute the check also from a fork?

See

. Feel free to provide PR for that.

@ZPascal ZPascal marked this pull request as ready for review February 21, 2022 21:28
@ZPascal
Copy link
Collaborator Author

ZPascal commented Feb 23, 2022

Hi @ad-m, I've checked on the GTE instance, if it's possible to use the API URL from the GitHub context. It's definitely the corresponding URL of the API Endpoint of the GitHub Enterprise system.

@ad-m
Copy link
Owner

ad-m commented Feb 23, 2022

@ZPascal could you rebase to include changes made in #109 to verify it?

@ZPascal
Copy link
Collaborator Author

ZPascal commented Feb 23, 2022

@ad-m Done. It doesn't seem to work yet. I will look at it in detail later.

@ZPascal
Copy link
Collaborator Author

ZPascal commented Feb 26, 2022

Hi @ad-m, I've implemented the general GTE support and tested it here. I also modified the GitHub Action to check the syntax inside the PR validation itself. I also tried to execute the real push action inside the PR, but that is not possible (The tokens got not the corresponding access rights to both repositories [forked and original]).

@ad-m
Copy link
Owner

ad-m commented Feb 28, 2022

I wonder if instead of introducing a new parameter to the action (intended purely for development purposes), it is not enough to write in the pull request template that you should look at the builds on the "push" event on the forking repository, instead of the "pull request" event on the current repository. GitHub-hosted forks should also have GitHub Actions running, so everything audible, so I don't see any reason not to. What do you think about this simplification?

@ZPascal
Copy link
Collaborator Author

ZPascal commented Apr 16, 2022

Hi @ad-m,

sounds validate. I'll modify the PR.

@ZPascal ZPascal requested a review from ad-m April 16, 2022 19:08
@ZPascal
Copy link
Collaborator Author

ZPascal commented Apr 29, 2022

Hi @ad-m, could we merge the PR?

@ad-m ad-m merged commit 694e694 into ad-m:master May 3, 2022
@ad-m
Copy link
Owner

ad-m commented May 3, 2022

LGTM

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