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

Add Julia formatter #1326

Merged
merged 1 commit into from
Mar 19, 2024
Merged

Add Julia formatter #1326

merged 1 commit into from
Mar 19, 2024

Conversation

simsurace
Copy link
Contributor

Maybe this will help a bit with readability.

@codecov-commenter
Copy link

codecov-commenter commented Mar 5, 2024

Codecov Report

Attention: Patch coverage is 71.51319% with 529 lines in your changes are missing coverage. Please review.

Project coverage is 74.51%. Comparing base (573ab89) to head (a4c62f3).

❗ Current head a4c62f3 differs from pull request most recent head 3cc48bd. Consider uploading reports for the commit 3cc48bd to get more accurate results

Files Patch % Lines
src/rules/llvmrules.jl 48.29% 76 Missing ⚠️
src/internal_rules.jl 12.34% 71 Missing ⚠️
src/compiler/optimize.jl 80.13% 60 Missing ⚠️
src/rules/typeunstablerules.jl 38.04% 57 Missing ⚠️
src/utils.jl 49.38% 41 Missing ⚠️
src/api.jl 80.00% 36 Missing ⚠️
src/compiler/utils.jl 75.47% 26 Missing ⚠️
src/rules/jitrules.jl 81.88% 25 Missing ⚠️
src/compiler/validation.jl 79.83% 24 Missing ⚠️
src/rules/customrules.jl 81.44% 18 Missing ⚠️
... and 13 more

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #1326       +/-   ##
===========================================
- Coverage   93.54%   74.51%   -19.04%     
===========================================
  Files           7       35       +28     
  Lines         248    10811    +10563     
===========================================
+ Hits          232     8056     +7824     
- Misses         16     2755     +2739     

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

@simsurace
Copy link
Contributor Author

There is an issue with permissions, probably requires admin permissions to fix.

julia -e 'using Pkg; Pkg.add("JuliaFormatter")'
julia -e 'using JuliaFormatter; format("."; verbose=true)'
- uses: reviewdog/action-suggester@v1
with:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
with:
with:
github_token: ${{ secrets.GITHUB_TOKEN }}

Copy link
Member

Choose a reason for hiding this comment

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

@simsurace did you intend to remove this when you force pushed?

examples/box.jl Outdated Show resolved Hide resolved
@vchuravy
Copy link
Member

Can we merge the Formatter on its own and slowly apply the suggested changes? Right now this will cause large conflicts with other PRs. I think clang-format can do this and for Python there is https://dev.to/akaihola/improving-python-code-incrementally-3f7a

x-ref: domluna/JuliaFormatter.jl#191

@simsurace
Copy link
Contributor Author

Sure thing.

@vchuravy
Copy link
Member

Thanks! Lets see how this does and if it is to annoying we may disable it.

@vchuravy vchuravy merged commit b6c02ba into EnzymeAD:main Mar 19, 2024
19 of 44 checks passed
@simsurace
Copy link
Contributor Author

How is it gonna work to get the codebase formatted now?

@vchuravy
Copy link
Member

As an example see #1302 where the changes themselves will be formatted. So slowly over time the code base will match the format rules, but without the cost of having large merge conflicts with existing PRs

@simsurace simsurace deleted the julia-formatter branch March 20, 2024 16:12
@simsurace
Copy link
Contributor Author

Ok. Might take a very long time to traverse the entire code base, but we could still format the other files separately. But e.g. in #1307 the action still fails due to permissions.

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.

3 participants