airflow-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jarek Potiuk <Jarek.Pot...@polidea.com>
Subject Re: [DISCUSS] Enable 'Black' for Auto Code Formatting
Date Sun, 28 Jun 2020 14:53:23 GMT
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 <Jarek.Potiuk@polidea.com>
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 <turbaszek@apache.org>
> 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 <ash@apache.org>
>> 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 <philgagnon1@gmail.com>
>> 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 <kaxilnaik@gmail.com>
>> 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: tomasz.urbaszek@polidea.com
>>
>> 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/>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message