-
Notifications
You must be signed in to change notification settings - Fork 63
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 GHES support #22
Add GHES support #22
Conversation
.github/workflows/labeler.yml
Outdated
enterprise-labeler: | ||
runs-on: ubuntu-latest | ||
name: Label the PR size for GHES support | ||
steps: | ||
- uses: actions/checkout@v1 | ||
- uses: ./ | ||
with: | ||
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
xs_max_size: '10' | ||
s_max_size: '50' | ||
m_max_size: '200' | ||
l_max_size: '450' | ||
fail_if_xl: 'true' | ||
message_if_xl: 'This PR is sooooo big!! 😳' | ||
github_api_url: 'api.github.com' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thx for the PR!
One question: Could we simplify this behaviour unifying it on the existing labeler
job only including the github_api_url
key?
That is, exposing the github_api_url
configuration parameter could be enough in order to leave it with api.github.com
as the default value in order to maintain the same behaviour as for now, and also provide backward compatibility in that sense, but also opening the option to override its value for GHES users.
Sorry if I am missing something. This is the first time we would be dealing with GHES and we could be not taking into account basic concepts 🙏
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah sure, I just had this fart of duplicating it. We can also use the fallback. I can give you immediate feedback if it works and otherwise I make the next PR. I am not yet 100% sure that I did all changes necessary for GHSE.
Please re-review! 🍻
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left the override to test the functionality.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🙌🙌🙌
Awesome work!
Merging…
Cool, let me go and test! Update, I can confirm it works with our GHES! 🥳 Thanks, @JavierCane for the fast support! |
Let me know if I interpreted your shell scripts correctly. Happy to make adjustments.