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

Use julia-format action #965

Merged
merged 4 commits into from
Jul 29, 2024

Conversation

JoshuaLampert
Copy link
Member

As discussed in #964 (comment) this uses the julia-format action, which makes suggestions in a PR based on the formatter instead of creating PRs. This should reduce the noise and guarantees that no non-formatting PRs are merged.
Note, however, that clickable suggestions in the PR are only made for PRs that don't come from forks. For PRs from forks only warnings with the corresponding diff from the formatter are shown (due to privilege issues).

@JoshuaLampert
Copy link
Member Author

Hm, I wanted to check if the annotations work, but unfortunately it looks like there are no annotations. I don't know why, especially because reviewdog promised to do so:

reviewdog: This GitHub Token doesn't have write permission of Review API [1],
so reviewdog will report results via logging command [2] and create annotations similar to
github-pr-check reporter as a fallback.

Copy link

codecov bot commented Jul 29, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.07%. Comparing base (5047961) to head (02a5d38).
Report is 2 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #965   +/-   ##
=======================================
  Coverage   89.07%   89.07%           
=======================================
  Files         178      183    +5     
  Lines        5483     5483           
=======================================
  Hits         4884     4884           
  Misses        599      599           

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

src/Meshes.jl Outdated Show resolved Hide resolved
@juliohm
Copy link
Member

juliohm commented Jul 29, 2024

@JoshuaLampert I see the format error showing up nicely.

Should we fix the code style and see the action turning green again?

@JoshuaLampert
Copy link
Member Author

@JoshuaLampert I see the format error showing up nicely.

...wait what exactly do you see? Do you see an annotation like here?
https://github.com/Astroshaper/AsteroidThermoPhysicalModels.jl/pull/107/files#diff-a17b585bea8f2c467215bd1821733e1580649126c74de4ff139ce6f019964762

Because I don't see that, but I would like to see it.

@juliohm
Copy link
Member

juliohm commented Jul 29, 2024

When I click on the Action that failed with the red cross mark, I see this:

image

It is bit hidden though.

@JoshuaLampert
Copy link
Member Author

Yes, I would have hoped for a bit more visibility and easier to read suggestions. In principle this should work like in the PR I linked above, but I don't know why this is not the case here. Maybe the bot needs some privileges?
But if you're fine with the current version, I can fix the code style again and we can merge.

@juliohm
Copy link
Member

juliohm commented Jul 29, 2024

I think this is already an improvement. We can experiment the new action and figure out missing permissions later. I will fix the code style and wait the tests again.

src/Meshes.jl Outdated Show resolved Hide resolved
@juliohm juliohm merged commit 9144833 into JuliaGeometry:master Jul 29, 2024
10 checks passed
@JoshuaLampert JoshuaLampert deleted the julia-format-action branch July 29, 2024 19:08
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