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

CI: add julia-format action #182

Merged

Conversation

alyst
Copy link
Contributor

@alyst alyst commented Apr 15, 2024

This adds the automatic format check via JuliaFormatter.jl and GitHub code suggestions using julia-format action.
This is an example of how it would look: alyst#2

Note that it would only suggest code formatting to the files modified by the PR, so to format the whole current codebase, a separate PR is required (it is as simple as running using JuliaFormatter; format(".") -- JuliaFormatter don't have to be a SEM.jl dependency, you can install it in the temporary Julia project; just the working folder should point to SEM.jl).
The formatting rules are specified in .JuliaFormatter.toml. Right now it is using the default formatting style.
Check the JuliaFormatter.jl docs for how to tune it to match to your preference.

I thought that it would be nice to fix the formatting before #181. One reason -- that PR contains many no-op whitespace changes (removal of trailing whitespace etc), which compilcate the review.

Fixes #169

Copy link

codecov bot commented Apr 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

❗ No coverage uploaded for pull request base (reformat@8cd36c3). Click here to learn what that means.

❗ Current head dcb5a76 differs from pull request most recent head 9ee30ba. Consider uploading reports for the commit 9ee30ba to get more accurate results

Additional details and impacted files
@@             Coverage Diff             @@
##             reformat     #182   +/-   ##
===========================================
  Coverage            ?   67.10%           
===========================================
  Files               ?       51           
  Lines               ?     2490           
  Branches            ?        0           
===========================================
  Hits                ?     1671           
  Misses              ?      819           
  Partials            ?        0           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Maximilian-Stefan-Ernst Maximilian-Stefan-Ernst changed the base branch from main to feature/formatter April 17, 2024 11:10
@Maximilian-Stefan-Ernst
Copy link
Collaborator

Nice, I now also added formatter to the test (so you are also told locally that you have to reformat before making a PR). Is there any reason why you are using julia-format@master instead of julia-format@v3?

@alyst
Copy link
Contributor Author

alyst commented Apr 17, 2024

Is there any reason why you are using julia-format@master instead of julia-format@v3?

No, I was just exploring the logic of this action. AFAIK, v3 is not yet released, but v2 should do the same thing, except it would not fail if it detects format violations outside of the lines affected by the PR.
I would revert to v2.

@Maximilian-Stefan-Ernst Maximilian-Stefan-Ernst changed the base branch from feature/formatter to reformat April 18, 2024 21:19
@Maximilian-Stefan-Ernst Maximilian-Stefan-Ernst merged commit ae6e408 into StructuralEquationModels:reformat Apr 18, 2024
2 checks passed
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 formatter
2 participants