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

trigger ci #10

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

trigger ci #10

wants to merge 6 commits into from

Conversation

jrhee17
Copy link
Owner

@jrhee17 jrhee17 commented Jun 7, 2023

Motivation:

Explain why you're making this change and what problem you're trying to solve.

Modifications:

  • List the modifications you've made in detail.

Result:

  • Closes #. (If this resolves the issue.)
  • Describe the consequences that a user will face after this PR is merged.

ikhoon pushed a commit to line/armeria that referenced this pull request Jun 8, 2023
Motivation:

Note that github secrets are not available for actions triggered by forked pull requests: [ref](https://github.blog/2020-08-03-github-actions-improvements-for-fork-and-pull-request-workflows/#improvements-for-public-repository-forks)

We have two choices
1. Using `pull_request` events:
- For push (merge) events, secrets are applied and the build cache is updated
- For pull requests from forked repositories, the build cache isn't updated. This means we can't take advantage of the build cache when checking out and reviewing pull requests locally.
- We can expect fewer cache uploaded, but higher hit rate
2. Using `pull_request_target` events:
- For all opened pull requests, secrets are applied and the build cache is updated
- For this type of event, the `.github` workflow files from the base commit is used. This means updating the `.github` workflow file can be tedious. (e.g. we need to merge changes we want to test to our forked repo, and then open a PR to verify changes)

I think it's fine to go with option 1 for now. Perhaps we can pivot to option 2 later depending on how much pressure our gradle enterprise instance is experiencing.
Unfortunately, the only way to test if this PR "works" is to push this commit to the `main` branch.

Test result from my forked repo:
- PR: jrhee17#10
- Build scan: https://ge.armeria.dev/s/hcdbl7vdcqbhc
- CI: https://github.com/jrhee17/armeria/actions/runs/5201312954/jobs/9381342458#step:5:1601

The options provided in `settings.gradle` are straightforward from Step 1 in the provided manual.

Modifications:

- Add a environment variable for gradle enterprise access key
- Add gradle enterprise configurations in `settings.gradle`

Result:

- Gradle enterprise integration
ikhoon pushed a commit to be-hase/armeria that referenced this pull request Jun 12, 2023
Motivation:

Note that github secrets are not available for actions triggered by forked pull requests: [ref](https://github.blog/2020-08-03-github-actions-improvements-for-fork-and-pull-request-workflows/#improvements-for-public-repository-forks)

We have two choices
1. Using `pull_request` events:
- For push (merge) events, secrets are applied and the build cache is updated
- For pull requests from forked repositories, the build cache isn't updated. This means we can't take advantage of the build cache when checking out and reviewing pull requests locally.
- We can expect fewer cache uploaded, but higher hit rate
2. Using `pull_request_target` events:
- For all opened pull requests, secrets are applied and the build cache is updated
- For this type of event, the `.github` workflow files from the base commit is used. This means updating the `.github` workflow file can be tedious. (e.g. we need to merge changes we want to test to our forked repo, and then open a PR to verify changes)

I think it's fine to go with option 1 for now. Perhaps we can pivot to option 2 later depending on how much pressure our gradle enterprise instance is experiencing.
Unfortunately, the only way to test if this PR "works" is to push this commit to the `main` branch.

Test result from my forked repo:
- PR: jrhee17#10
- Build scan: https://ge.armeria.dev/s/hcdbl7vdcqbhc
- CI: https://github.com/jrhee17/armeria/actions/runs/5201312954/jobs/9381342458#step:5:1601

The options provided in `settings.gradle` are straightforward from Step 1 in the provided manual.

Modifications:

- Add a environment variable for gradle enterprise access key
- Add gradle enterprise configurations in `settings.gradle`

Result:

- Gradle enterprise integration
@jrhee17 jrhee17 force-pushed the main branch 2 times, most recently from 3f1c4c8 to eb3e012 Compare June 28, 2023 06:49
@jrhee17 jrhee17 force-pushed the main branch 5 times, most recently from 1a21a66 to f94002a Compare July 30, 2023 04:43
@jrhee17 jrhee17 force-pushed the main branch 4 times, most recently from 934b219 to d907eb4 Compare August 24, 2023 14:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant