Do we need a script though? They would only need to rebase their PR on master and the pre-commit hook will update their code with "black" too
On Sun, Jun 28, 2020 at 3:53 PM Jarek Potiuk <[email protected]> wrote: > And one more thing - such script/tool might be a nice tool to give for > people doing rebases of their PRs from pre-black to post-black. This way we > will make it easier for people to rebase their changes after black. I think > it should simply take all the commits that we are reading and apply > black formatting to every commit being rebased. This should make an easy > path for people to switch. We can prepare such a script and test it now - > rebasing some of the PRs in progress just to test it - and even test some > cherry-picking after that. I am happy to help with making the tool and > testing it. > > J. > > On Sun, Jun 28, 2020 at 12:08 PM Jarek Potiuk <[email protected]> > wrote: > > > Agree with Black and agree with Ash and Tomek about cherry-pickiness. .. > > But I also think this is the right time to do it. > > > > After 1.10.11 we will have some cherry-picks to do for 1.10.12 > > undoubtedly. However I presume we will be much closer to releasing 2.0 > and > > we will not cherry-pick any of the past changes (let's make sure in > > 1.10.11rc1 we will cherry-pick everything that we have to from the > master). > > Also the 1.10.12 changes will be more of bug-fixes rather than bringing > any > > of the old features in. > > > > Then I think it is crucial to run Black formatting at the same time in > > v1-10-test/v1-10-stable and in master (right after we release 1.10.11 > rc1). > > This way any "fixes" that should be cherry-picked to 1.10.11rc2 etc will > > use black for both - master and 1.10.11. > > > > If we do it at the same time, then cherry-picking all the future changes > > should not be a problem. I think the problem with cherry-picking will > only > > happen if we try to cherry-pick pre-black change to post-black code. And > > even then I think we can do some clever automation. I am sure we can > easily > > write a simple script to extract the commit to cherry-pick, apply black > and > > only then apply the cherry-picked commit. That should be rather easy to > do > > - and it will enable us to cherry-pick both pre-black and post-black > > commits. > > > > I am willing to spend more time on cherry-picking in 1.10.12 - I think > > benefits of having an uncompromising code formatter are much more "time > for > > review" saving than "time for cherry-picking" adding. > > > > J, > > > > On Sun, Jun 28, 2020 at 11:10 AM Tomasz Urbaszek <[email protected]> > > wrote: > > > >> This is a good idea, and for new code in Airflow I usually use black. > >> However, I agree with Ash that this can make cherrypicking harder even > >> if we run black on v1-10-test branch (there are already differences in > >> code). Personally I would be in favor of introducing black and other > >> things (i.e. pyupgrade) when releasing 2.0. > >> > >> Regarding the issue of merge conflict in PR - we will not be able to > >> avoid it (however running black locally before rebase should help I > >> think). > >> > >> Cheers, > >> Tomek > >> > >> > >> On Sun, Jun 28, 2020 at 12:51 AM Ash Berlin-Taylor <[email protected]> > >> wrote: > >> > > >> > Yes, to Black in principle. But (and this is a big but) no, not yet. > Or > >> not without testing how it affects our ability to cherry pick back to > the > >> release branch. > >> > > >> > (My default would be to assume it makes them harder/almost impossible > >> and this should be the almost last thing we do before we release 2.0) > >> > > >> > -ash > >> > > >> > On 27 June 2020 22:02:41 BST, Philippe Gagnon <[email protected]> > >> wrote: > >> > >It's a good idea. > >> > > > >> > >It will make reading the codebase easier, and besides the whole > Python > >> > >ecosystem is slowly moving towards adopting this code style. I > >> > >personally > >> > >have been a fan ever since the project launched. > >> > > > >> > >With regards to open PRs requiring a rebase, it's an annoyance for > sure > >> > >but > >> > >if we do decide to standardize on any code style (which we should, > >> > >black or > >> > >not), we'll have to pull the bandaid eventually. > >> > > > >> > >Just my two cents. ;-) > >> > > > >> > >Philippe > >> > > > >> > >On Sat, Jun 27, 2020 at 4:13 PM Kaxil Naik <[email protected]> > >> wrote: > >> > > > >> > >> Hi all, > >> > >> > >> > >> I would like to open the discussion to enable Black ( > >> > >> https://github.com/psf/black) - *The Uncompromising Code > Formatter* > >> > >for > >> > >> automatic formatting of Airflow's entire codebase. > >> > >> > >> > >> I have created a WIP PR at > >> > >https://github.com/apache/airflow/pull/9550 > >> > >> > >> > >> Some of the caveats: > >> > >> > >> > >> - All the currently open PRs would have some kind of conflict > >> > >errors > >> > >> - It *might *make backporting harder (but should be ok'ish if we > >> > >enable > >> > >> black on v1-10-test but not 100%) > >> > >> - There are known issues with "line-lengths" not being honoured > by > >> > >black > >> > >> (https://github.com/psf/black/issues/1161) and the workaround > is > >> > >to use > >> > >> "#fmt: off". > >> > >> > >> > >> I would love to hear what the community thinks about it. > >> > >> > >> > >> Regards, > >> > >> Kaxil > >> > >> > >> > >> > >> > >> -- > >> > >> Tomasz Urbaszek > >> Polidea | Software Engineer > >> > >> M: +48 505 628 493 > >> E: [email protected] > >> > >> Unique Tech > >> Check out our projects! > >> > > > > > > -- > > > > Jarek Potiuk > > Polidea <https://www.polidea.com/> | Principal Software Engineer > > > > M: +48 660 796 129 <+48660796129> > > [image: Polidea] <https://www.polidea.com/> > > > > > > -- > > Jarek Potiuk > Polidea <https://www.polidea.com/> | Principal Software Engineer > > M: +48 660 796 129 <+48660796129> > [image: Polidea] <https://www.polidea.com/> >
