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

[Automated PR] Bump KTLint and/or Google Java Formatter #67

Merged
merged 3 commits into from
Jun 1, 2021

Conversation

macisamuele
Copy link
Owner

Bump KTLint and/or Google Java Formatter

Auto-generated by bump-releases

@macisamuele macisamuele enabled auto-merge May 31, 2021 00:51
@codecov
Copy link

codecov bot commented May 31, 2021

Codecov Report

Merging #67 (1aaa0a4) into master (dff1c5e) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master       #67   +/-   ##
=========================================
  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...1aaa0a4. Read the comment docs.

@cortinico
Copy link
Collaborator

For some reason tests are failing on windows. Do you happen to know why @macisamuele ?

@macisamuele
Copy link
Owner Author

I'll be looking into the windows failures.
The bright side is that tests did got executed for the automated PR (so at least #62 worked)

@macisamuele
Copy link
Owner Author

macisamuele commented May 31, 2021

@cortinico from a very superficial (possibly still incorrect) analysis I see that windows tests are failing because:

  • ktlint requires relative paths, but it does look for paths with the forward-slash (/) and in windows the path separator is \ (example in here).
    I have to check how in real pre-commit executions the path would be passed in to understand if this is testing issue or this would break during real execution as well
  • even assuming that we can replace \ with / (here), tests would continue to fail because seems that ktlint does not write the fixed file even if --autofix is passed in.

Considering the above findings, I'm going to look more into this hoping to identify what is the root cause of this and hoping that it is testing setup rather than a big regression in ktlint.

Edit: After a little debugging I've found out what is happening in here

  1. ktlint does not accept \ (on Windows) in the input paramters
  2. ktlint uses \ (on Windows) paths
  3. the tool uses the ktlint output to decide on which files to run the formatter (using then the \ as provided by ktling, which does not work :( )

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 force-pushed the create-pull-request/bump-releases branch from 4dd0729 to 1aaa0a4 Compare May 31, 2021 13:03
@macisamuele macisamuele disabled auto-merge May 31, 2021 13:03
@macisamuele
Copy link
Owner Author

Manually rebased PR on top of #68 .
Please let's merge the 2 PRs separately.

@macisamuele macisamuele enabled auto-merge May 31, 2021 20:18
@macisamuele macisamuele requested a review from cortinico May 31, 2021 20:18
@macisamuele macisamuele merged commit b6b0784 into master Jun 1, 2021
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