Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
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. 🚀 New features to boost your workflow:
|
cf37495 to
4815f38
Compare
|
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 |
|
For me it is important to keep formatting in the same (squashed) PR, otherwise 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 |
There was a problem hiding this comment.
Should the bot now have created a commit here?
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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).
|
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. |
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. |
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. |
|
Ey - I was doing the review here and did not even have time to look at the answers before this was merged. |
|
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. |
|
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. |
Each commit is reasonably selfcontained:
@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.