Skip to content

Conversation

@juliohm
Copy link
Member

@juliohm juliohm commented Nov 1, 2025

I believe this is a typo, but didn't check the source code. It doesn't make sense to have a default cost of 1.0 when the user assigns the value 0.0.

@codecov
Copy link

codecov bot commented Nov 1, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.16%. Comparing base (9a7a6df) to head (9305f0c).

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #469   +/-   ##
=======================================
  Coverage   97.16%   97.16%           
=======================================
  Files         121      121           
  Lines        7020     7020           
=======================================
  Hits         6821     6821           
  Misses        199      199           

☔ 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
Copy link
Member

In this piece of code you seem to be correct https://github.com/JuliaGraphs/Graphs.jl/blob/master/src/traversals/maxadjvisit.jl#L37-L49

However, the mere presence of this third point makes sense only if your correction is wrong. If your correction is right, we should just remove this line or at least restructure it. Otherwise, with the correction it reads "when you do A, surprisingly A does happen".

@gdalle , git blame says you wrote this line in the docs. Could you comment on this edit? I try to be careful with Chesterton's fence

@gdalle
Copy link
Member

gdalle commented Nov 9, 2025

@gdalle , git blame says you wrote this line in the docs. Could you comment on this edit? I try to be careful with Chesterton's fence

This must be from when I formatted the whole repo with JuliaFormatter (and didn't know about git blame yet). Sorry I can't help out here, I don't know the traversal code base well. For shortest paths what @juliohm wrote would be true indeed

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