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

Handle latest ktlint on windows #68

Merged
merged 2 commits into from
May 31, 2021

Conversation

macisamuele
Copy link
Owner

In the #66 we tried to address a new limitation/regression introduced by the latest ktlint version.
Once then the automated PR #67 was created we noticed that windows tests started to fail.

A more complete analysis is present in #67 (comment)

The TL;DR; is

  • ktlint does not accept paths like src\file.kt (which is the correct way of describing relative paths on Windows platform
  • ktlint does provide. paths like src\file.kt in the output reporting (which is odd)

This PR aims to work-the-issue-around by replacing \ with / in the paths.

NOTE: This PR does also make some improvements on the typing of the codebase.

WARNING: ktlint is intentionally not bumped on this PR because I still want to make sure that tests are going to be green with 0.40.0.
Once this will be merged I'll be re-triggering tests of #67 so we can also ensure that they are going to be green (I did already run them locally on a Windows computer)

This is caused by the fact that KTLint==0.41.0 has altered the way that paths, from CLI, are read and apparently they dropped support for absolute paths as well as for `\` in the Windows path.

This commit aims to set a workaround for it
@macisamuele macisamuele requested a review from cortinico May 31, 2021 11:58
@codecov
Copy link

codecov bot commented May 31, 2021

Codecov Report

Merging #68 (b0e3e6c) into master (dff1c5e) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master       #68   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           10        10           
  Lines          318       321    +3     
=========================================
+ Hits           318       321    +3     
Impacted Files Coverage Δ
...ormatters_pre_commit_hooks/pretty_format_kotlin.py 100.00% <100.00%> (ø)
..._formatters_pre_commit_hooks/pretty_format_yaml.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dff1c5e...b0e3e6c. Read the comment docs.

Copy link
Collaborator

@cortinico cortinico left a comment

Choose a reason for hiding this comment

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

Thanks for the explanation 👍 LGTM

@macisamuele macisamuele merged commit 66df2f0 into master May 31, 2021
@macisamuele macisamuele deleted the maci-handle-latest-ktlint-on-windows branch July 18, 2021 16:41
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.

2 participants