Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix git-sync-init resources indent #35824

Merged
merged 1 commit into from Nov 24, 2023

Conversation

romsharon98
Copy link
Collaborator

Fixes: #35815


^ 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.

Copy link
Member

@hussein-awala hussein-awala left a comment

Choose a reason for hiding this comment

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

Could you add a test that reproduces the problem you have?

@eladkal eladkal changed the title change indent to 4 Fix git-sync-init resources indent Nov 23, 2023
@eladkal eladkal added this to the Airflow Helm Chart 1.12.0 milestone Nov 23, 2023
Copy link
Contributor

@jscheffl jscheffl left a comment

Choose a reason for hiding this comment

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

Change looks OK for me.
Tests on this are actually hard, because tests are made by parsing YAML and doing logic tests. This is "just cosmetic" and adjusts the indent. It removed un-needed spacing.
For me good to be merged.

Copy link
Contributor

@eladkal eladkal left a comment

Choose a reason for hiding this comment

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

I agree with @jscheffl
Wrong indent can be on every line its not so simple to cover.

WDYT @hussein-awala ?

@hussein-awala
Copy link
Member

I agree with @jscheffl

Wrong indent can be on every line its not so simple to cover.

WDYT @hussein-awala ?

Okay, I will try to add a test later.

@hussein-awala hussein-awala merged commit 39107df into apache:main Nov 24, 2023
47 checks passed
ephraimbuddy pushed a commit that referenced this pull request Nov 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:helm-chart Airflow Helm Chart
Projects
None yet
Development

Successfully merging this pull request may close these issues.

git-sync-init resources is too indented
4 participants