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

Revert "fix: migrate to 0.49.0" #41

Closed
wants to merge 1 commit into from
Closed

Conversation

ghaiszaher
Copy link
Member

Reverts #40

It might be that the new flag is ineffective and --android still works, so we need to roll back to avoid breaking changes.

@ghaiszaher ghaiszaher requested a review from a team as a code owner April 27, 2023 10:04
@ghaiszaher ghaiszaher mentioned this pull request Apr 27, 2023
Comment on lines -40 to -53
ktlint_version=$(ktlint --version)
echo "ktlint version: $ktlint_version"

if [ "$(printf '%s\n' "0.49.0" "$ktlint_version" | sort -V | head -n1)" = "0.49.0" ]; then
if [ "$INPUT_ANDROID" = true ]; then
export ANDROID="--code-style=android_studio"
else
export ANDROID="--code-style=intellij_idea"
fi
else
if [ "$INPUT_ANDROID" = true ]; then
export ANDROID=--android
fi
fi

Choose a reason for hiding this comment

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

On one hand --code-style=android_studio is what ktlint is saying to use with ktlint 0.49.0 but apparently this paramter has no effect (neither on CI, nor locally)

On the other hand --android is throwing error

ERROR com.pinterest.ktlint.cli.internal.KtlintCommandLine - Option '--android' / '-a' is deprecated and replaced with option '--code-style=android_studio'. Setting '.editorconfig' property 'ktlint_code_style=android_studio' might be a better idea for a project that is always to formatted with this code style.

but still seems to work just fine.

If this --code-style parameter really does not work with ktlint 0.49.0 (which was released few days ago) I would expect 0.49.1 to appear soon, meaning it would make more sense probably to keep action-ktlint as is hoping that problems will be solved soon on ktlint side

Copy link
Member Author

Choose a reason for hiding this comment

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

I raised an issue pinterest/ktlint#1982
Let's see if it can get fixed soon.

Copy link
Member Author

Choose a reason for hiding this comment

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

Issue will be fixed in next ktlint version. I will close the PR and raise another one to apply these changes starting v0.49.1

@ghaiszaher
Copy link
Member Author

Replaced by #41

@ghaiszaher ghaiszaher closed this May 5, 2023
@ghaiszaher ghaiszaher deleted the revert-40-master branch May 5, 2023 13:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants