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

added short paragraph in docs on the dangers of deleting vertices #179

Merged
merged 2 commits into from
May 1, 2023

Conversation

henrik-wolf
Copy link
Contributor

I recently found the solution to a bug in my code related to the persistence of vertex indices when deleting earlier vertices in the graph. Since this behaviour of "renaming" later vertices to fill the spots of deleted ones is (at least from my point of view) fairly unexpected, I decided to contribute a little paragraph in the documentation.
Let me know what you think about it.
(also... am I requesting the right pull target? Do I need to build something for this to work?)

@codecov
Copy link

codecov bot commented Oct 9, 2022

Codecov Report

Merging #179 (88de075) into master (1783617) will decrease coverage by 0.25%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #179      +/-   ##
==========================================
- Coverage   97.47%   97.23%   -0.25%     
==========================================
  Files         109      114       +5     
  Lines        6349     6584     +235     
==========================================
+ Hits         6189     6402     +213     
- Misses        160      182      +22     

gdalle
gdalle previously requested changes Feb 9, 2023
docs/src/first_steps/access.md Outdated Show resolved Hide resolved
@gdalle
Copy link
Member

gdalle commented Feb 9, 2023

Thanks @SuperGrobi for the suggestion, indeed it is a very frequent pain point so mentioning it in the docs will help lots of people

@henrik-wolf
Copy link
Contributor Author

Thank you very much for the review! I find the whole github-interface somewhat unintuitive, so just to be sure that I am not missing anything substantial: do I need to do something here to accept the requested changes? Or has that to be done by another reviewer with elevated access?

@gdalle
Copy link
Member

gdalle commented May 1, 2023

do I need to do something here to accept the requested changes? Or has that to be done by another reviewer with elevated access?

In this instance I took care of it, but you could also have accepted the modifications because they happen on your branch, not on the main repo. The only time you need someone with merge access is to incorporate your changes into the official code

@gdalle gdalle dismissed their stale review May 1, 2023 05:58

Outdated

@gdalle gdalle merged commit e99fc6a into JuliaGraphs:master May 1, 2023
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.

None yet

2 participants