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 missing related PRs when cherry-picking #33261

Merged

Conversation

potiuk
Copy link
Member

@potiuk potiuk commented Aug 9, 2023

The airflow-github tooling had a check if PR has been already merged, but the PR did not work well when there was already a PR cherry-picked that referred the PR in question in their log message.

The check was done via git log --grep=#PR essentially and so it happenes that --grep matches not only the first line of the message but the whole log message. So if the log message contained reference to the cherry-picked PR it was seen as merged.

This change adds ( ) around and $ to the regexp to match not only the PR number but also parentheses around.


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

The `airflow-github` tooling had a check if PR has been already
merged, but the PR did not work well when there was already a PR
cherry-picked that referred the PR in question in their log message.

The check was done via `git log --grep=#PR` essentially and so
it happenes that `--grep` matches not only the first line of
the message but the whole log message. So if the log message
contained reference to the cherry-picked PR it was seen as merged.

This change adds ( ) around and $ to the regexp to match not
only the PR number but also parentheses around.
@ephraimbuddy ephraimbuddy merged commit eb07618 into apache:main Aug 9, 2023
ephraimbuddy pushed a commit that referenced this pull request Aug 9, 2023
The `airflow-github` tooling had a check if PR has been already
merged, but the PR did not work well when there was already a PR
cherry-picked that referred the PR in question in their log message.

The check was done via `git log --grep=#PR` essentially and so
it happenes that `--grep` matches not only the first line of
the message but the whole log message. So if the log message
contained reference to the cherry-picked PR it was seen as merged.

This change adds ( ) around and $ to the regexp to match not
only the PR number but also parentheses around.

(cherry picked from commit eb07618)
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
…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 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)
@potiuk potiuk deleted the fix-missed-prs-when-cherry-picking branch November 17, 2023 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants