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

Test gha #1

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

Test gha #1

wants to merge 4 commits into from

Conversation

Will-Lo
Copy link
Owner

@Will-Lo Will-Lo commented Dec 11, 2024

Summary

Issue] Briefly discuss the summary of the changes made in this
pull request in 2-3 lines.

Changes

  • Client-facing API Changes
  • Internal API Changes
  • Bug Fixes
  • New Features
  • Performance Improvements
  • Code Style
  • Refactoring
  • Documentation
  • Tests

For all the boxes checked, please include additional details of the changes made in this pull request.

Testing Done

  • Manually Tested on local docker setup. Please include commands ran, and their output.
  • Added new tests for the changes made.
  • Updated existing tests to reflect the changes made.
  • No tests added or updated. Please explain why. If unsure, please feel free to ask for help.
  • Some other form of testing like staging or soak time in production. Please explain.

For all the boxes checked, include a detailed description of the testing done for the changes made in this pull request.

Additional Information

  • Breaking Changes
  • Deprecations
  • Large PR broken into smaller PRs, and PR plan linked in the description.

For all the boxes checked, include additional details of the changes made in this pull request.

Will-Lo added a commit to linkedin/openhouse that referenced this pull request Dec 17, 2024
…date the correct commit hashes (#269)

## Summary

This PR adds a new workflow which largely reuses the existing workflow
for building/creating images, except that it runs on pull requests but
also uses the correct hashes. The PR workflow also has some
optimizations to cancel the previous test run on the same PR if a new
commit is pushed since that test result is outdated.

<!--- HINT: Replace #nnn with corresponding Issue number, if you are
fixing an existing issue -->

[Issue](https://github.com/linkedin/openhouse/issues/#nnn)] Briefly
discuss the summary of the changes made in this
pull request in 2-3 lines.

For all the boxes checked, please include additional details of the
changes made in this pull request.

The workflow on the main branch appears to be using the merge commit SHA
for tagging purposes: https://github.com/anothrNick/github-tag-action

However this causes issues where it can use outdated hashes of pull
requests due to using `github.event.pull_request.merge_commit_sha` which
does not seem consistent with the Pull Request HEAD SHA.

Documentation on the default behavior here that should fix it
https://github.com/actions/checkout/blob/main/README.md

This can cause PRs to have breaking commits but due to this workflow
file can pass the CI tests, making it difficult to work in the
repository. Another side effect of the PR is that committers branches
that have multiple commits may have difficulty debugging their failing
workflows locally since the CI tests are running on outdated files.

See testing below for replication of the issue.

## Changes

- [ ] Client-facing API Changes
- [ ] Internal API Changes
- [x] Bug Fixes
- [ ] New Features
- [ ] Performance Improvements
- [ ] Code Style
- [ ] Refactoring
- [ ] Documentation
- [ ] Tests


## Testing Done
<!--- Check any relevant boxes with "x" -->

- [ ] Manually Tested on local docker setup. Please include commands
ran, and their output.
- [ ] Added new tests for the changes made.
- [ ] Updated existing tests to reflect the changes made.
- [ ] No tests added or updated. Please explain why. If unsure, please
feel free to ask for help.
- [x] Some other form of testing like staging or soak time in
production. Please explain.

For all the boxes checked, include a detailed description of the testing
done for the changes made in this pull request.

Tested locally on my repo
Will-Lo#1 shows a PR with the old
behavior which has a clearly breaking change, but is passing tests since
the commit that opened the PR passed tests

Will-Lo#3 shows a PR that used to pass
with the commit that opened it, but now fails with the same breaking
change.

# Additional Information

- [ ] Breaking Changes
- [ ] Deprecations
- [ ] Large PR broken into smaller PRs, and PR plan linked in the
description.

For all the boxes checked, include additional details of the changes
made in this pull request.
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.

1 participant