Hey Everyone,

TL;DR; I planned to ask for help in reviewing the 48 (pretty much
fully automated PRs I prepared, but a comment from TP made me think that
maybe I went too much ahead and maybe there are other ideas on implementing
the change (or even maybe not implementing it at all).

More context:

I have just attempted what I THOUGHT was generally accepted and good idea,
but comment from TP made me think maybe I went ahead of everyone else, so I
am reaching here to see if this was a good idea (And I am ready to back-out
and do it differently - and follow another way if someone can propose it).

I just (literally while watching Poland <-> Brazil Volleybal World
Championship semi-finals) prepared 48 separate PRs to migrate to
__future__.annotations which (I thought) was considered a good idea and
something we wanted to defer to after v2-4-test branch cut-off because of
the concerns that it might make cherry-picking more complex. It was 9X%
fully automated conversion - by search/replace and adding `from __future__
import annotations` and letting upgrade do the job (as discussed before).
I also split the one huge PR into 48 (!) much smaller PRs - basically one
per each "airflow" core package, few big providers separately, tests
separately (as they are far less risky than "core code" change).

We already added __future__ annotation in a number of other places before -
without a "broad" agreement and consensus/voting (different people did in
various places - including myself and others) so I thought this is not
something we need a broader voting/consensus on.

Again - that was 95% automated change where I contributed mostly the
thinking "how to split '' theI PR and all the rest was literally "git
commit add <package>" (and let pyupgrade and our other pre-commits like
isort and others) do the job.

It never occured to me that this might be a problem for anyone. My
understanding of the problem was:
* we want to do it
* we were afraid to do it before because it would make cherry-picking to
2-3 branch more problematic
* we wanted to do it right "when" we cut-off the 2.4 branch (for the same
reason as above - we wanted to avoid cherry-picking problems for 2.4.*
changes)

So I wanted to make this change as fast as humanly possible - and raise
this multiple PRs so we can review/approve (and I'd cherry-pick them) as
soon as possible to minimise the cherry-picking problem.

But maybe I was too fast and it's not as straightforward as I thought it
was? (actually it never occured to me that there might be a slightest
problem with it after the earlier discussion). I literally thought about it
as a mechanical change that we want to introduce and that doing that in
small chunks (as discussed in the original PR) was the best approach. The
comment from TP
https://apache-airflow.slack.com/archives/CCPRP7943/p1662837864413849  -
made me think that I have misjudged it.

I wanted to write that email asking all of the committers here to help to
review and merge all those PRs as fast as possible (again - those are
purely mechanical changes, there are literally a few - literally less than
5) manual changes I had to make to fix some mypy problems.
All the PRs are here:
https://github.com/apache/airflow/pulls?q=is%3Aopen+is%3Apr+label%3Afuture-annotations

But then I ask for something else then:

* do you think this is fine we add __future__ annotations in general
* do you think the way I proposed it is somewhat too In-human (i honestly
feel it is an in-human change - because it is fully automated by our tools
:D). But is it wrong approach to have 48 PRs automatically prepared by our
tools to get a "consistent" approach
* did I do it too fast/too early ?
* any ideas on how we can do it differently to avoid the "cherry-picking"
problems (mentioned by others in the original message)?

I am really concerned to ask if this is something I have done wrongly or
misunderstood the consensus, and maybe chose the wrong way ? Maybe I should
approach it differently and ask for consensus on such a flurry of PRS? Or
maybe the timing is wrong.

I am happy to follow in any way there - I can either keep the PRs and
rebase them until the time is good (it takes about two minutes to rebase
every one of them and apply the automation again)  if the way I proposed it
was because I misunderstood the "sentiment" about the change - or follow
any other way that will help us to implement the change - or even abandon
it completely if we will agree it's not a good idea. I am also happy to
apply any "general comments" there - now that the PR's are there, applying
any automation even on all of them to improve it, is rather simple.

Looking forward to any comments. I am really curious how others look at it
- regardless of the outcome - always eager to learn new things :)

J.














On Thu, Jul 7, 2022 at 8:52 AM Tobiasz Kędzierski <
tobiaszkedzier...@gmail.com> wrote:

> Great idea.  +1 from me. Thanks for raising this up Jarek and thanks to
> Pierre for great isort tip :)
>
> BR
> Tobiasz
>

Reply via email to