Skip to content
This repository was archived by the owner on Dec 8, 2023. It is now read-only.

feat: airflow modern syntax migration#35

Merged
morgante merged 4 commits intogetgrit:mainfrom
srijan-paul:airflow
Oct 24, 2023
Merged

feat: airflow modern syntax migration#35
morgante merged 4 commits intogetgrit:mainfrom
srijan-paul:airflow

Conversation

@srijan-paul
Copy link
Contributor

@srijan-paul srijan-paul commented Oct 23, 2023

A migration that moves airflow legacy syntax to the newer decorator syntax.

/claim #31

@srijan-paul srijan-paul marked this pull request as ready for review October 24, 2023 06:32
$call <: collect_kwargs($kwargs),
$decorator_args = join(list=$kwargs, separator=", "),
$task_func => `@task($decorator_args)\n$task_func`,
// $call => $name
Copy link
Contributor

Choose a reason for hiding this comment

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

No commented out code please.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed.

@srijan-paul srijan-paul requested a review from morgante October 24, 2023 15:17
$function <: dag_definition($var) as $decl,
$dag_refs = [],
$function <: contains bubble ($function, $dag_refs) identifier() as $dag_ref where {
$dag_ref <: after $decl,
Copy link
Contributor

Choose a reason for hiding this comment

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

What's this line meant to do? $decl is unbound, so this doesn't really check anything.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whoops, I realized this pattern wasn't used.
it's from an older impl that I got rid of.
Removed it.

Copy link
Contributor

@morgante morgante left a comment

Choose a reason for hiding this comment

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

Looks good to start. I will need to do some more testing to validate it works successfully on real repos.

@morgante morgante merged commit 3a41767 into getgrit:main Oct 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants