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

Add LollipopGraph #417

Merged
merged 42 commits into from
Mar 12, 2021
Merged

Add LollipopGraph #417

merged 42 commits into from
Mar 12, 2021

Conversation

marinaanagno
Copy link
Contributor

No description provided.

@samanthaharper samanthaharper self-requested a review February 24, 2021 14:32
Copy link
Contributor

@samanthaharper samanthaharper left a comment

Choose a reason for hiding this comment

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

I think this is perfect, except that it should return an immutable digraph, so you need to update that and update the documentation.

doc/examples.xml Outdated Show resolved Hide resolved
gap/examples.gi Show resolved Hide resolved
@james-d-mitchell james-d-mitchell added the new-feature A label for new features. label Feb 28, 2021
Marina Anagnostopoulou-Merkouri added 2 commits March 2, 2021 19:50
doc/examples.xml Outdated Show resolved Hide resolved
@wilfwilson
Copy link
Collaborator

wilfwilson commented Mar 3, 2021

Currently one of the PR status checks is failing: the new 'codecov/patch' one. This means you've added or changed code that isn't correspondingly covered by tests.

You can see the bits of your added/changed code that don't have code coverage by following the "Details" link next to the failing codecov/patch PR status check.

In particular, that link will tell you "Added line #L402 was not covered by tests".

You can also see this information by looking for 'codecov' comments on the diff: https://github.com/digraphs/Digraphs/pull/417/files

gap/examples.gi Outdated Show resolved Hide resolved
marinaanagno and others added 8 commits March 10, 2021 14:42
Co-authored-by: Wilf Wilson <wilf@wilf-wilson.net>
Co-authored-by: Wilf Wilson <wilf@wilf-wilson.net>
Co-authored-by: Wilf Wilson <wilf@wilf-wilson.net>
Co-authored-by: Wilf Wilson <wilf@wilf-wilson.net>
Co-authored-by: Wilf Wilson <wilf@wilf-wilson.net>
@digraphs digraphs deleted a comment from codecov bot Mar 11, 2021
Copy link
Collaborator

@wilfwilson wilfwilson left a comment

Choose a reason for hiding this comment

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

I think this is looking good! As with the other PRs, there will unfortunately be merge conflicts to resolve when you next update with the master branch. Let me know if that's trouble.

Otherwise, I have some requests about the documentation, and some suggestions for further properties to set about the lollipop graph.

doc/examples.xml Outdated Show resolved Hide resolved
gap/examples.gi Show resolved Hide resolved
@wilfwilson wilfwilson changed the title add lollipop graph Add lollipop graph Mar 11, 2021
@wilfwilson wilfwilson changed the title Add lollipop graph Add LollipopGraph Mar 11, 2021
wilfwilson
wilfwilson previously approved these changes Mar 12, 2021
Copy link
Collaborator

@wilfwilson wilfwilson left a comment

Choose a reason for hiding this comment

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

I've fixed some things that got broken by the merge conflicts, and also I addressed my remaining requested changes from #417 (comment) which you had already marked as resolved 😉 . I think this can be merged if the tests pass for a final time.

@marinaanagno
Copy link
Contributor Author

I've fixed some things that got broken by the merge conflicts, and also I addressed my remaining requested changes from #417 (comment) which you had already marked as resolved 😉 . I think this can be merged if the tests pass for a final time.

Ooops I'm so sorry! I don't know how I did this! Apologies! :)
Thank you so much for all this help!

@wilfwilson wilfwilson merged commit d13f945 into digraphs:master Mar 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new-feature A label for new features.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants