You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2019/12/30 00:20:57 UTC
[GitHub] [airflow] mik-laj opened a new pull request #6960: [AIRFLOW-XXX]
Add tips for writing a note in UPDATIND.md
mik-laj opened a new pull request #6960: [AIRFLOW-XXX] Add tips for writing a note in UPDATIND.md
URL: https://github.com/apache/airflow/pull/6960
Hi.
I think writing notes is a problem for developers, and it's not always helpful enough for end-users. I wrote a few short points that can improve future notes. This is a very important document when it comes to experience when migrating to the new version, so we should ensure that users enjoy reading it.
> - [ ] Previous behaviors
A good description of the previous behavior allows you to check if the problem is specific to you
> - [ ] New behaviors
It allows you to understand what has changed.
- [ ] If possible, a simple example of how to migrate. This may include a simple code example.
People love examples. It's the easiest way to understand what you have to do.
- [ ] If possible, the benefit for the user after migration e.g. "we want to make these changes to unify class names."
People don't like change. If they get the benefit they are willing to make changes.
- [ ] If possible, the reason for the change, which adds more context to that interested, e.g. reference for Airflow Improvment Proposal.
A short note does not always allow you to understand the whole change. A reference to another document will help you better understand the change and will also involve new people in the community.
Make sure you have checked _all_ steps below.
### Jira
- [ ] My PR addresses the following [Airflow Jira](https://issues.apache.org/jira/browse/AIRFLOW/) issues and references them in the PR title. For example, "\[AIRFLOW-XXX\] My Airflow PR"
- https://issues.apache.org/jira/browse/AIRFLOW-XXX
- In case you are fixing a typo in the documentation you can prepend your commit with \[AIRFLOW-XXX\], code changes always need a Jira issue.
- In case you are proposing a fundamental code change, you need to create an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)).
- In case you are adding a dependency, check if the license complies with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
### Description
- [ ] Here are some details about my PR, including screenshots of any UI changes:
### Tests
- [ ] My PR adds the following unit tests __OR__ does not need testing for this extremely good reason:
### Commits
- [ ] My commits all reference Jira issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)":
1. Subject is separated from body by a blank line
1. Subject is limited to 50 characters (not including Jira issue reference)
1. Subject does not end with a period
1. Subject uses the imperative mood ("add", not "adding")
1. Body wraps at 72 characters
1. Body explains "what" and "why", not "how"
### Documentation
- [ ] In case of new functionality, my PR adds documentation that describes how to use it.
- All the public functions and the classes in the PR contain docstrings that explain what it does
- If you implement backwards incompatible changes, please leave a note in the [Updating.md](https://github.com/apache/airflow/blob/master/UPDATING.md) so we can assign it to a appropriate release
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #6960: [AIRFLOW-XXX]
Add tips for writing a note in UPDATIND.md
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6960: [AIRFLOW-XXX] Add tips for writing a note in UPDATIND.md
URL: https://github.com/apache/airflow/pull/6960#issuecomment-569557767
# [Codecov](https://codecov.io/gh/apache/airflow/pull/6960?src=pr&el=h1) Report
> Merging [#6960](https://codecov.io/gh/apache/airflow/pull/6960?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/5e100ab753635521d33f287bdffd3d9bd4c6ae0f?src=pr&el=desc) will **decrease** coverage by `0.22%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/airflow/pull/6960?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6960 +/- ##
==========================================
- Coverage 84.7% 84.47% -0.23%
==========================================
Files 679 679
Lines 38488 38488
==========================================
- Hits 32600 32514 -86
- Misses 5888 5974 +86
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6960?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/operators/mysql\_operator.py](https://codecov.io/gh/apache/airflow/pull/6960/diff?src=pr&el=tree#diff-YWlyZmxvdy9vcGVyYXRvcnMvbXlzcWxfb3BlcmF0b3IucHk=) | `0% <0%> (-100%)` | :arrow_down: |
| [airflow/operators/mysql\_to\_hive.py](https://codecov.io/gh/apache/airflow/pull/6960/diff?src=pr&el=tree#diff-YWlyZmxvdy9vcGVyYXRvcnMvbXlzcWxfdG9faGl2ZS5weQ==) | `0% <0%> (-100%)` | :arrow_down: |
| [airflow/utils/sqlalchemy.py](https://codecov.io/gh/apache/airflow/pull/6960/diff?src=pr&el=tree#diff-YWlyZmxvdy91dGlscy9zcWxhbGNoZW15LnB5) | `89.83% <0%> (-6.78%)` | :arrow_down: |
| [airflow/hooks/hive\_hooks.py](https://codecov.io/gh/apache/airflow/pull/6960/diff?src=pr&el=tree#diff-YWlyZmxvdy9ob29rcy9oaXZlX2hvb2tzLnB5) | `76.08% <0%> (-1.53%)` | :arrow_down: |
| [airflow/models/connection.py](https://codecov.io/gh/apache/airflow/pull/6960/diff?src=pr&el=tree#diff-YWlyZmxvdy9tb2RlbHMvY29ubmVjdGlvbi5weQ==) | `67.8% <0%> (-0.98%)` | :arrow_down: |
| [airflow/hooks/dbapi\_hook.py](https://codecov.io/gh/apache/airflow/pull/6960/diff?src=pr&el=tree#diff-YWlyZmxvdy9ob29rcy9kYmFwaV9ob29rLnB5) | `90.9% <0%> (-0.83%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6960?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6960?src=pr&el=footer). Last update [5e100ab...d29c657](https://codecov.io/gh/apache/airflow/pull/6960?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] mik-laj commented on issue #6960: [AIRFLOW-XXX] Add tips
for writing a note in UPDATIND.md
Posted by GitBox <gi...@apache.org>.
mik-laj commented on issue #6960: [AIRFLOW-XXX] Add tips for writing a note in UPDATIND.md
URL: https://github.com/apache/airflow/pull/6960#issuecomment-569614216
I think that the migration procedure will be best developed by the person who creates the change. She/He is aware of all the consequences. It may be very difficult for someone to understand all the edge cases. A good description will allow us to write the tool easier. If we also have detailed descriptions, it will be easier for us to combine entries. It's best to start creating the migration procedure now. In the future it will be easier for us to improve than to build it from scratch.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io commented on issue #6960: [AIRFLOW-XXX] Add
tips for writing a note in UPDATIND.md
Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #6960: [AIRFLOW-XXX] Add tips for writing a note in UPDATIND.md
URL: https://github.com/apache/airflow/pull/6960#issuecomment-569557767
# [Codecov](https://codecov.io/gh/apache/airflow/pull/6960?src=pr&el=h1) Report
> Merging [#6960](https://codecov.io/gh/apache/airflow/pull/6960?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/5e100ab753635521d33f287bdffd3d9bd4c6ae0f?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/airflow/pull/6960?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6960 +/- ##
======================================
Coverage 84.7% 84.7%
======================================
Files 679 679
Lines 38488 38488
======================================
Hits 32600 32600
Misses 5888 5888
```
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6960?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6960?src=pr&el=footer). Last update [5e100ab...d29c657](https://codecov.io/gh/apache/airflow/pull/6960?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] nuclearpinguin commented on issue #6960: [AIRFLOW-XXX]
Add tips for writing a note in UPDATIND.md
Posted by GitBox <gi...@apache.org>.
nuclearpinguin commented on issue #6960: [AIRFLOW-XXX] Add tips for writing a note in UPDATIND.md
URL: https://github.com/apache/airflow/pull/6960#issuecomment-569622336
> I think that the migration procedure will be best developed by the person who creates the change. She/He is aware of all the consequences. It may be very difficult for someone to understand all the edge cases.
This assumes that all authors are still active contributors and remember everything about the change. This is a really strong assumption imho. But that's where a good description should help.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] potiuk commented on issue #6960: [AIRFLOW-XXX] Add tips
for writing a note in UPDATIND.md
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #6960: [AIRFLOW-XXX] Add tips for writing a note in UPDATIND.md
URL: https://github.com/apache/airflow/pull/6960#issuecomment-569599081
I think it is far too many details at this stage (especially for 2.0 where we accumulate changes and there might contradicting or overlapping changes to do. We might revert some changes after we test it or we might want to combine some of the changes in which case there will be completely different migration procedure.
I believe we should add all the details (especially migration procedure) before releasing 2.0.0 - when we will be writing the migration tool described in https://issues.apache.org/jira/browse/AIRFLOW-6390.
This will be the right time to provide examples and test the migration process I think.
Also we have the past entries which do not have all the details anyway, so we will have to do the exercise anyway.
For me current level of details we have in UPDATING.md was enough.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] kaxil merged pull request #6960: [AIRFLOW-XXX] Add tips
for writing a note in UPDATIND.md
Posted by GitBox <gi...@apache.org>.
kaxil merged pull request #6960: [AIRFLOW-XXX] Add tips for writing a note in UPDATIND.md
URL: https://github.com/apache/airflow/pull/6960
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] nuclearpinguin commented on a change in pull request
#6960: [AIRFLOW-XXX] Add tips for writing a note in UPDATIND.md
Posted by GitBox <gi...@apache.org>.
nuclearpinguin commented on a change in pull request #6960: [AIRFLOW-XXX] Add tips for writing a note in UPDATIND.md
URL: https://github.com/apache/airflow/pull/6960#discussion_r361881391
##########
File path: UPDATING.md
##########
@@ -41,6 +41,21 @@ assists users migrating to a new version.
## Airflow Master
+<!--
+
+I'm glad you want to write a new note. Make sure it contains the following information:
Review comment:
```suggestion
I'm glad you want to write a new note. Remember that this note is intended for users.
Make sure it contains the following information:
```
I would emphasise that those notes are mostly for users.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] mik-laj commented on issue #6960: [AIRFLOW-XXX] Add tips
for writing a note in UPDATIND.md
Posted by GitBox <gi...@apache.org>.
mik-laj commented on issue #6960: [AIRFLOW-XXX] Add tips for writing a note in UPDATIND.md
URL: https://github.com/apache/airflow/pull/6960#issuecomment-569623544
This is the reason why I want to create a migration procedure now. Other people may stop contributing to the project and creating this procedure will be difficult. Existing entries are another problem that requires a different solution.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #6960: [AIRFLOW-XXX]
Add tips for writing a note in UPDATIND.md
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #6960: [AIRFLOW-XXX] Add tips for writing a note in UPDATIND.md
URL: https://github.com/apache/airflow/pull/6960#issuecomment-569557767
# [Codecov](https://codecov.io/gh/apache/airflow/pull/6960?src=pr&el=h1) Report
> Merging [#6960](https://codecov.io/gh/apache/airflow/pull/6960?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/5e100ab753635521d33f287bdffd3d9bd4c6ae0f?src=pr&el=desc) will **decrease** coverage by `0.22%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/airflow/pull/6960?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6960 +/- ##
==========================================
- Coverage 84.7% 84.47% -0.23%
==========================================
Files 679 679
Lines 38488 38488
==========================================
- Hits 32600 32514 -86
- Misses 5888 5974 +86
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/6960?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/operators/mysql\_operator.py](https://codecov.io/gh/apache/airflow/pull/6960/diff?src=pr&el=tree#diff-YWlyZmxvdy9vcGVyYXRvcnMvbXlzcWxfb3BlcmF0b3IucHk=) | `0% <0%> (-100%)` | :arrow_down: |
| [airflow/operators/mysql\_to\_hive.py](https://codecov.io/gh/apache/airflow/pull/6960/diff?src=pr&el=tree#diff-YWlyZmxvdy9vcGVyYXRvcnMvbXlzcWxfdG9faGl2ZS5weQ==) | `0% <0%> (-100%)` | :arrow_down: |
| [airflow/utils/sqlalchemy.py](https://codecov.io/gh/apache/airflow/pull/6960/diff?src=pr&el=tree#diff-YWlyZmxvdy91dGlscy9zcWxhbGNoZW15LnB5) | `89.83% <0%> (-6.78%)` | :arrow_down: |
| [airflow/hooks/hive\_hooks.py](https://codecov.io/gh/apache/airflow/pull/6960/diff?src=pr&el=tree#diff-YWlyZmxvdy9ob29rcy9oaXZlX2hvb2tzLnB5) | `76.08% <0%> (-1.53%)` | :arrow_down: |
| [airflow/models/connection.py](https://codecov.io/gh/apache/airflow/pull/6960/diff?src=pr&el=tree#diff-YWlyZmxvdy9tb2RlbHMvY29ubmVjdGlvbi5weQ==) | `67.8% <0%> (-0.98%)` | :arrow_down: |
| [airflow/hooks/dbapi\_hook.py](https://codecov.io/gh/apache/airflow/pull/6960/diff?src=pr&el=tree#diff-YWlyZmxvdy9ob29rcy9kYmFwaV9ob29rLnB5) | `90.9% <0%> (-0.83%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/6960?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/6960?src=pr&el=footer). Last update [5e100ab...d29c657](https://codecov.io/gh/apache/airflow/pull/6960?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] potiuk commented on issue #6960: [AIRFLOW-XXX] Add tips
for writing a note in UPDATIND.md
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #6960: [AIRFLOW-XXX] Add tips for writing a note in UPDATIND.md
URL: https://github.com/apache/airflow/pull/6960#issuecomment-569657386
Ok. Let's try it - no harm in that. I already updated the description in #6950. Let's see if others will follow.
What do you think we should do with the entries that have too short description from the past @mik-laj ? Do you want to ask authors of those changes to update the descriptions or you want to update it now ? or wait for release and update it there?
IMHO - If you look at your point above:
> This is the reason why I want to create a migration procedure now. Other people may stop contributing to the project and creating this procedure will be difficult. Existing entries are another problem that requires a different solution.
the natural consequence is to follow up with the authors and ask them to provide missing information. Does it make sense? Are you willing to do that @mik-laj ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [airflow] potiuk edited a comment on issue #6960: [AIRFLOW-XXX] Add
tips for writing a note in UPDATIND.md
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on issue #6960: [AIRFLOW-XXX] Add tips for writing a note in UPDATIND.md
URL: https://github.com/apache/airflow/pull/6960#issuecomment-569599081
I think it is far too many details at this stage (especially for 2.0 where we accumulate changes and there might contradicting or overlapping changes to do. We might revert some changes after we test it or we might want to combine some of the changes in which case there will be completely different migration procedure.
I believe we should add all the details (especially migration procedure) before releasing 2.0.0 - when we will be writing the migration tool described in https://issues.apache.org/jira/browse/AIRFLOW-6390.
This will be the right time to provide examples and test the migration process I think.
Also we have the past entries which do not have all the details anyway, so we will have to do the exercise anyway.
For me current level of details we have in UPDATING.md was enough for now..
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services