Skip to content

Conversation

@Krastanov
Copy link
Member

@Krastanov Krastanov commented Jun 3, 2025

Each commit is reasonably selfcontained:

  • add an autoformat bot that will create a commit in this same PR with formatting fixes
  • if a PR is pushed to, stop the CI running on old commits, to avoid burning our free credits
  • use a more recent simpler version of the cache action
  • add a misformat error on purpose to test the bot
  • add a PR template explaining the presence of the bot
  • the automated linter commit that was generated by the bot, fixing my on-purpose injected mistake

@rafaqz @simonschoelly , let me know if this fits your needs, I would like to merge it soon so we can remove one of the burdens putting off new contributors.

@codecov
Copy link

codecov bot commented Jun 3, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.41%. Comparing base (c001dab) to head (c213702).
Report is 2 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #437   +/-   ##
=======================================
  Coverage   97.41%   97.41%           
=======================================
  Files         120      120           
  Lines        6953     6953           
=======================================
  Hits         6773     6773           
  Misses        180      180           

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Krastanov Krastanov force-pushed the sk/ciformatter branch 3 times, most recently from cf37495 to 4815f38 Compare June 3, 2025 17:44
@Krastanov Krastanov closed this Jun 3, 2025
@Krastanov Krastanov reopened this Jun 3, 2025
@Krastanov Krastanov changed the title [WIP] draft autoformat bot autoformat bot in CI Jun 3, 2025
@Krastanov Krastanov requested review from rafaqz and simonschoelly June 3, 2025 18:47
@rafaqz
Copy link
Contributor

rafaqz commented Jun 4, 2025

I thought it would be more like compat helper that makes a PR with format updates, but this seems good too if it doesn't block PRs and has the same effect

@Krastanov
Copy link
Member Author

For me it is important to keep formatting in the same (squashed) PR, otherwise git blame would become unusable. And git blame is probably the most important tool when supporting a codebase built by someone else, as I am doing here.

I will wait a day or two for any dissent and merge this.

Return a vector containing the indegrees of every vertex of the graph `g`, where
the indegree of a vertex is defined as the number of edges which end at that
the indegree of a vertex is defined as the number of edges which end at that
Copy link
Member

@simonschoelly simonschoelly Jun 5, 2025

Choose a reason for hiding this comment

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

Should the bot now have created a commit here?

Copy link
Member Author

Choose a reason for hiding this comment

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

I guess not -- this was fixed by my IDE auto-save, not by JuliaFormatter.

I just checked and the repo is actually littered with trailing whitespaces:

> git checkout master; git pull
# what follows is a command that removes trailing whitespaces that I found on stack overflow
> (export LANG=C LC_CTYPE=C
find . -not \( -name .svn -prune -o -name .git -prune \) -type f -print0 | perl -0ne 'print if -T' | xargs -0 sed -Ei 's/[[:blank:]]+$//'
)
> git status
...
	modified:   Experimental/Traversals/bfs.jl
	modified:   Experimental/isomorphism.jl
	modified:   Parallel/vertexcover/random_vertex_cover.jl
	modified:   SimpleGraphs/generators/randgraphs.jl
	modified:   SimpleGraphs/simpledigraph.jl
	modified:   centrality/degree.jl
	modified:   centrality/eigenvector.jl
	modified:   centrality/pagerank.jl
	modified:   centrality/radiality.jl
	modified:   community/assortativity.jl
	modified:   community/modularity.jl
	modified:   connectivity.jl
	modified:   core.jl
	modified:   dominatingset/degree_dom_set.jl
	modified:   graphcut/karger_min_cut.jl
	modified:   independentset/degree_ind_set.jl
	modified:   independentset/maximal_ind_set.jl
	modified:   iterators/bfs.jl
	modified:   iterators/dfs.jl
	modified:   shortestpaths/bellman-ford.jl
	modified:   shortestpaths/floyd-warshall.jl
	modified:   shortestpaths/johnson.jl
	modified:   shortestpaths/yen.jl
	modified:   spanningtrees/kruskal.jl
	modified:   steinertree/steiner_tree.jl
	modified:   traversals/all_simple_paths.jl
	modified:   traversals/bfs.jl
	modified:   trees/prufer.jl
	modified:   vertexcover/random_vertex_cover.jl

I will leave this mystery of why whitespaces are not cleaned up for another PR. Seems to be a status-quo issue, not a new one introduced by this PR.


The linter bot might add new commits to your PR (e.g. to take care of formatting issues). Feel free to overwrite these commits.

If the linter bot is the last one to put in a commit, **THE TESTS WILL NOT RUN**. Either lint/format your contributions yourself as described in CONTRIBUTING.md or close&reopen the PR to reset the test runner. No newline at end of file
Copy link
Member

Choose a reason for hiding this comment

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

Is it not possible to have the tests run after the linter? As we have seen with JuliaFormatter v2 there is the possibility that the formatter introduces errors.

Copy link
Member Author

Choose a reason for hiding this comment

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

It is possible, but I do not think that would fix the main issue -- if the bot makes a commit, tests will not run afterwards because github actions do not get triggered by bots and the PR status will not report the old test (you would need to go in the actions tab and scroll to past commits).

@simonschoelly
Copy link
Member

Let's try this out to see if we gain some benefits with that. An other option would be just tell people to add a precommit hook to their git repository that triggers JuliaFormatter.

@Krastanov
Copy link
Member Author

An other option would be just tell people to add a precommit hook

My goal with this PR is to avoid asking new contributors to do anything nonstandard, because that would significantly lower the amount of improvements being submitted.

@Krastanov
Copy link
Member Author

Let's try this out to see if we gain some benefits with that

Thanks! I will go ahead and merge this and will be on the lookout on new PRs if something breaks. I will be available for cleanup if the bot ends up being unpleasant to use.

@Krastanov Krastanov merged commit 8efd394 into master Jun 5, 2025
13 checks passed
@Krastanov Krastanov deleted the sk/ciformatter branch June 5, 2025 01:28
@simonschoelly
Copy link
Member

Ey - I was doing the review here and did not even have time to look at the answers before this was merged.

@Krastanov
Copy link
Member Author

Apologies, I understood "Let's try this out" as a go ahead for merge. I can revert this and resubmit?

@simonschoelly
Copy link
Member

Apologies, I understood "Let's try this out" as a go ahead for merge. I can revert this and resubmit?

Sorry, I did not see that now. I think we need to disable the bot as long as we don't have a way to make it work from forks.

@Krastanov
Copy link
Member Author

Sounds good! I will make a revert PR and later this week I will submit a revert-revert with a fix for PRs coming from forks.

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.

4 participants