Closed saeranv closed 4 years ago
@chriswmackey I'll add some unit tests to get that coverage back up for these changes. Sorry for not doing it before adding the functionality!
Also, @saeranv , I think that coverage result isn't accurate (there might be something in your commit history that's throwing it off). More tests are always appreciated but I think we can ignore what coveralls is telling us right now (hopefully, it should self-correct after we merge).
@chriswmackey these changes should address everything.
@saeranv ,
Your addressing of the comments look good but this PR now includes the polyskel.py
file (among several other files that don't belong like the extra __init__
file that I mentioned earlier and .vscode/settings
). Maybe you should just clone the ladybug_geometry_polyskel
repo to your machine and separate what you have into the two repos locally.
@chriswmackey Argh, yeah I had this set correctly and then I think I messed it up by trying to merge my local branch and master, and then not looking over it. Apologies, this latest commit should be good.
Thank you for fixing this @saeranv . Let's merge it now. I left a comment over on your ladybug_geometry_polyskel
PR about something that I think should be moved over here to ladybug_geometry
but we can add it with another PR after this is merged.
:tada: This PR is included in version 1.12.0 :tada:
The release is available on:
v1.12.0
Your semantic-release bot :package::rocket:
These are the changes to ladybug-geometry that are required for polyskel, without any code for the polyskel.py