Skip to content

Comments

Re-order pre-commit checks so that the FastAPI spec is generated before UI client#47408

Merged
ashb merged 1 commit intomainfrom
generate-api-spec-before-ts-client
Mar 5, 2025
Merged

Re-order pre-commit checks so that the FastAPI spec is generated before UI client#47408
ashb merged 1 commit intomainfrom
generate-api-spec-before-ts-client

Conversation

@ashb
Copy link
Member

@ashb ashb commented Mar 5, 2025

Otherwise you have to run pre-commit twice in order to get all changes "fully" done.

…re UI client

Otherwise you have to run pre-commit twice in order to get all changes "fully" done.
Copy link
Collaborator

@aritra24 aritra24 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the * removed in the docs? Seems inconsequential so lgtm

@ashb ashb merged commit 14a74ca into main Mar 5, 2025
44 checks passed
@jedcunningham jedcunningham deleted the generate-api-spec-before-ts-client branch March 5, 2025 20:45
@potiuk
Copy link
Member

potiuk commented Mar 6, 2025

Nice :)

@potiuk
Copy link
Member

potiuk commented Mar 6, 2025

However, this one is not good for now - because the generate-openapi-spec is a breeze tests - it runs inside the CI image (not sure if that is needed BTW) - and we have all the breeze tests at the end - also because they are skipped when breeze image is not built - to speed up static checks - so we should rather move the other test after generate test.

@potiuk
Copy link
Member

potiuk commented Mar 6, 2025

I will do it in a moment

potiuk added a commit to potiuk/airflow that referenced this pull request Mar 6, 2025
The open-api generation (for now at least) uses the CI image to
run, so it should be after "mypy" checks in order to be skippable
autonatically for very simple changes (for example when README
only changes - there we run only checks that do not need CI
image, and the CI-image bound ones are automatically skipped).

The apache#47408 moved the generate fastapi spec before the compiling
(which made sense) but also removed the fastapi generation from
the "CI" section of pre-commit.

This PR moves both tests to the CI section - which is not a
problem because if UI compilation is needed that always
triggers full CI static checks.
@potiuk
Copy link
Member

potiuk commented Mar 6, 2025

#47461 as follow up

potiuk added a commit that referenced this pull request Mar 6, 2025
…47461)

The open-api generation (for now at least) uses the CI image to
run, so it should be after "mypy" checks in order to be skippable
autonatically for very simple changes (for example when README
only changes - there we run only checks that do not need CI
image, and the CI-image bound ones are automatically skipped).

The #47408 moved the generate fastapi spec before the compiling
(which made sense) but also removed the fastapi generation from
the "CI" section of pre-commit.

This PR moves both tests to the CI section - which is not a
problem because if UI compilation is needed that always
triggers full CI static checks.
nailo2c pushed a commit to nailo2c/airflow that referenced this pull request Apr 4, 2025
nailo2c pushed a commit to nailo2c/airflow that referenced this pull request Apr 4, 2025
…pache#47461)

The open-api generation (for now at least) uses the CI image to
run, so it should be after "mypy" checks in order to be skippable
autonatically for very simple changes (for example when README
only changes - there we run only checks that do not need CI
image, and the CI-image bound ones are automatically skipped).

The apache#47408 moved the generate fastapi spec before the compiling
(which made sense) but also removed the fastapi generation from
the "CI" section of pre-commit.

This PR moves both tests to the CI section - which is not a
problem because if UI compilation is needed that always
triggers full CI static checks.
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.

8 participants