Skip to content

Conversation

@ephraimbuddy
Copy link
Contributor

AIP-50 was omitted in the changelog

@ephraimbuddy ephraimbuddy requested review from ashb and potiuk May 2, 2023 09:57
Co-authored-by: Pankaj Koti <pankajkoti699@gmail.com>

New Features
^^^^^^^^^^^^
- AIP-50 Trigger DAG UI Extension with Flexible User Form Concept (`AIP-50 <https://github.com/apache/airflow/pulls?q=is%3Apr+is%3Amerged+label%3AAIP-50+milestone%3A%22Airflow+2.6.0%22>`_)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can just show #27063
the bug fix of #29376 can be skipped from change log because it's not a fix that relevant for the users. it's something found and addressed before release.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For me, since the link above shows the 2 PRs, it's better than showing one and skipping the other. Both will have credits for contributing to the change

Copy link
Member

@potiuk potiuk May 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. I think having two prs explicitly mentioned there is the only way to go. Something like (#27063, #29376).

The problem with labels is that they are not persistent. I can imagine we do cleanup in GitHub and remove old unused labels a year from now. Should it make more difficult to see which PRs were part of the change when users look for it in 2 years ?

We are deliberately using PR# instead of ISSUE# (and have the rule that we do not have to have the issue) precisely for the reason to keep it persistently stored in our git history. Every single PR# we merge and display here is not only something that will link you directly to the PR on GitHub, but it is also always (our squash & merge workflow guarantees that) part of the commit message of the merged PR.

This is important for the longevity of the Foundation PMCs. As an ASF PMC we have to be prepared to leave GitHub if needs be. If we decide tomorrow that we move to GitLab for example, we should be able to move our git repo there and the context should move with us (our git is mirrored in GitBox service that is run by the ASF infrastructure). So if the release notes will say (#27063,#29376), then you will be able to find the very commits it refers by searching commit messaes in GitLab. In such case we will have no idea whatsoever what is%3Apr+is%3Amerged+label%3AAIP-50+milestone%3A%22Airflow+2.6.0%22 is all about.

This is also why I try to promote and encourage putting a lot of context in the commit messages and why we move decision making from GitHub issues to devlist for important matters.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have updated it.

Copy link
Member

@potiuk potiuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to add PR# (see the comment why)

@ephraimbuddy ephraimbuddy requested a review from potiuk May 2, 2023 18:03
@ephraimbuddy ephraimbuddy merged commit ea18edb into apache:main May 2, 2023
@ephraimbuddy ephraimbuddy deleted the add-aip-50 branch May 2, 2023 18:26
ephraimbuddy added a commit that referenced this pull request May 2, 2023
* Add missing changelog in 2.6.0

* Update RELEASE_NOTES.rst

Co-authored-by: Pankaj Koti <pankajkoti699@gmail.com>

* fixup! Add missing changelog in 2.6.0

---------

Co-authored-by: Pankaj Koti <pankajkoti699@gmail.com>
(cherry picked from commit ea18edb)
@ephraimbuddy ephraimbuddy added this to the Airflow 2.6.1 milestone May 8, 2023
@ephraimbuddy ephraimbuddy added the changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) label May 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

4 participants