-
Notifications
You must be signed in to change notification settings - Fork 11
Use libsonata to resolve node sets #213
Conversation
joni-herttuainen
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@GianlucaFicarelli Tell me what you think. If this looks somewhat ok, I'll update the docs and add changelog entries. Should we bump this up a major version bearing in mind we will also change {nodes,edges}.get?
Codecov Report
@@ Coverage Diff @@
## master #213 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 29 29
Lines 2163 2159 -4
=========================================
- Hits 2163 2159 -4
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Co-authored-by: Gianluca Ficarelli <[email protected]>
|
Things look reasonable to me, but I haven't been following it as closely as @GianlucaFicarelli - IMO he should have the final say. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
Should we release 1.0.7 before merging and releasing 1.1.0?
Alternatively, the changelog should be updated (moving the entries from 1.0.7 to 1.1.0, after rebasing on master)
|
I'm happy with merging and releasing any none-breaking changes first |
I'd like to merge #220 once it's approved (since it's not a breaking change), release 1.0.7, then merge this PR and release 1.1.0. What do you think? |
|
Sounds good. I think #220 is good to go. |
GianlucaFicarelli
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
No description provided.