Skip to content

Conversation

@potiuk
Copy link
Member

@potiuk potiuk commented Jul 11, 2023

So far, only list of flags and their details have been used to determine if the screenshots of Breeze commands should be regenerated (by calculating hash of those options). However also rich-click options determine the look of those screenshots (by grouping options together) so change in the options should also be included in hash generation.

This change combines the dict of click options for each command with the rich-click options into a single dictionary and hashes them together - this way any time any of those parameters change, the image will need to be regenerated.


^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

@potiuk
Copy link
Member Author

potiuk commented Jul 11, 2023

Small life quality improvements for those who modify / add new breeze images @amoghrajesh -> this one will ask you to regenerate images for #32495 (previously it would not ask to regenerate screenshots if you have only updated command_*_configy.py. Small thing but it will prevent commiting not-updated screenshots. I've been meaning to do it before, but I figured I can add it now so that you can test it on your change (after we merge it and you rebase on top of it).

@potiuk
Copy link
Member Author

potiuk commented Jul 11, 2023

BTW. The changed images are because I have not yet figured out the differences on Mac and Linux. There is a slight difference between the screenshots generated on those which I have no idea where they come from - this is not a big issue and it's ok to get some images generated on either, but maybe some day we will fix it. All the images willl now generated on Linux after this one is merged.

@potiuk potiuk requested a review from hussein-awala July 11, 2023 12:17
@potiuk potiuk force-pushed the add-option-groups-to-command-hashes branch 2 times, most recently from 5297570 to 59ad2f5 Compare July 11, 2023 12:24
So far, only list of flags and their details have been used to determine
if the screenshots of Breeze commands should be regenerated (by
calculating hash of those options). However also rich-click options
determine the look of those screenshots (by grouping options together)
so change in the options should also be included in hash generation.

This change combines the dict of click options for each command with
the rich-click options into a single dictionary and hashes them
together - this way any time any of those parameters change, the image
will need to be regenerated.
@potiuk potiuk force-pushed the add-option-groups-to-command-hashes branch from 59ad2f5 to f0ba6af Compare July 11, 2023 12:25
@potiuk
Copy link
Member Author

potiuk commented Jul 11, 2023

And nice error in case you add new command and it is missing in rich-click options:

image

@potiuk potiuk merged commit 50ca5c1 into apache:main Jul 11, 2023
@potiuk potiuk deleted the add-option-groups-to-command-hashes branch July 11, 2023 15:03
potiuk added a commit to potiuk/airflow that referenced this pull request Jul 15, 2023
The recent "add-back-references" command was not added to group
to nicely show it in `release-management --help` output. This
PR fixes it, and at the same time it turned out that similarly
as in subcommands fixed in apache#32523, hash of command did  not include
options, so when **just** options were changed, the images were not
marked as needing regeneration.

Adding options to hash calculation solves the problem.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants