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

do not write formatted string to file at all if overwrite==false #364

Merged
merged 2 commits into from
Feb 12, 2021

Conversation

baumgold
Copy link
Contributor

Closes #363

@domluna
Copy link
Owner

domluna commented Feb 10, 2021

bin/format.jl hasn't been updated in a while, IMO it should be deleted.

Doesn't https://github.com/julia-actions/julia-format/blob/master/workflows/format_check.yml#L34-L42 do the same thing?

@baumgold
Copy link
Contributor Author

bin/format.jl hasn't been updated in a while, IMO it should be deleted.

Sounds good to me.

Doesn't https://github.com/julia-actions/julia-format/blob/master/workflows/format_check.yml#L34-L42 do the same thing?

Yes but it only works for CI systems that use Github Actions. I think a generic tool that can be called from any CI system would be useful.

@domluna
Copy link
Owner

domluna commented Feb 10, 2021

ic - well that snippet could be copied over, GHA is not a requirement for it.

@baumgold
Copy link
Contributor Author

ic - well that snippet could be copied over, GHA is not a requirement for it.

That snippet depends on calling format(".", verbose=true) on L32 which modifies the files in-place. My goal here is to be able to call format(".", overwrite=false) to check whether the files are formatted correctly without modifying the files.

@domluna
Copy link
Owner

domluna commented Feb 12, 2021

Ok that makes sense - the current overwrite functionality is not useful anyway so this looks good.

Can you update the docs here

If `overwrite` is `true` the file will be reformatted in place, overwriting
the existing file; if it is `false`, the formatted version of `foo.jl` will
not be written anywhere.

nvm brain fell asleep lol

@domluna domluna merged commit 56759a3 into domluna:master Feb 12, 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.

Add feature to check if code is formatted correctly
2 participants