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

Fix getting correct commit from multiple referenced PR #33411

Merged
merged 1 commit into from
Aug 15, 2023

Conversation

ephraimbuddy
Copy link
Contributor

When a PR is referenced by other PRs, our dev tool for getting the correct commit lists the latest commit when looking for the commit sha but we should get the oldest.

If you run:
git log --grep "#33145" --format=%H returns two commits:

3dd0c999f1159a2fefbf32d9f10208a274a79a62
d1d6fc994d46aaed9c801162595cae91a1ffc19c

The correct commit for the PR is d1d6fc994d46aaed9c801162595cae91a1ffc19c but the first one is returned.

This PR uses --reverse to get the oldest commit where the PR number #33145 was mentioned which equals the PR.

When a PR is referenced by other PRs, our dev tool for getting the correct
commit lists the latest commit when looking for the commmit sha but we should get the oldest.
@ephraimbuddy ephraimbuddy merged commit 5b104a9 into apache:main Aug 15, 2023
1 check passed
@ephraimbuddy ephraimbuddy deleted the multiple-ref-pr branch August 15, 2023 17:35
potiuk added a commit to potiuk/airflow that referenced this pull request Aug 15, 2023
This is a more complete fix to apache#33411. This is also a follow up on
earlier implementation of apache#33261 that addressed checking if PRs
are merged. This one applies the same pattern to finding commit
but also improves it by checking if the (#NNNNNN) ends the subject
- so even if the PR is in the same form in the message, it will be
filtered out.

The previous "--reverse" quick fix in apache#33411 had potential of problem in
case there were releated PRs merged before the original PR (which is
quite posssible when you have a series of PRs referring to each other.
potiuk added a commit that referenced this pull request Aug 15, 2023
…3418)

This is a more complete fix to #33411. This is also a follow up on
earlier implementation of #33261 that addressed checking if PRs
are merged. This one applies the same pattern to finding commit
but also improves it by checking if the (#NNNNNN) ends the subject
- so even if the PR is in the same form in the message, it will be
filtered out.

The previous "--reverse" quick fix in #33411 had potential of problem in
case there were releated PRs merged before the original PR (which is
quite posssible when you have a series of PRs referring to each other.
ferruzzi pushed a commit to aws-mwaa/upstream-to-airflow that referenced this pull request Aug 17, 2023
When a PR is referenced by other PRs, our dev tool for getting the correct
commit lists the latest commit when looking for the commmit sha but we should get the oldest.
ferruzzi pushed a commit to aws-mwaa/upstream-to-airflow that referenced this pull request Aug 17, 2023
…ache#33418)

This is a more complete fix to apache#33411. This is also a follow up on
earlier implementation of apache#33261 that addressed checking if PRs
are merged. This one applies the same pattern to finding commit
but also improves it by checking if the (#NNNNNN) ends the subject
- so even if the PR is in the same form in the message, it will be
filtered out.

The previous "--reverse" quick fix in apache#33411 had potential of problem in
case there were releated PRs merged before the original PR (which is
quite posssible when you have a series of PRs referring to each other.
@ephraimbuddy ephraimbuddy added this to the Airflow 2.7.1 milestone Aug 27, 2023
@ephraimbuddy ephraimbuddy added the changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) label Aug 27, 2023
ephraimbuddy added a commit that referenced this pull request Aug 28, 2023
When a PR is referenced by other PRs, our dev tool for getting the correct
commit lists the latest commit when looking for the commmit sha but we should get the oldest.

(cherry picked from commit 5b104a9)
ephraimbuddy pushed a commit that referenced this pull request Aug 28, 2023
…3418)

This is a more complete fix to #33411. This is also a follow up on
earlier implementation of #33261 that addressed checking if PRs
are merged. This one applies the same pattern to finding commit
but also improves it by checking if the (#NNNNNN) ends the subject
- so even if the PR is in the same form in the message, it will be
filtered out.

The previous "--reverse" quick fix in #33411 had potential of problem in
case there were releated PRs merged before the original PR (which is
quite posssible when you have a series of PRs referring to each other.

(cherry picked from commit 3766ab0)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:dev-tools changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants