Closed alicekwak closed 3 years ago
Merging #216 (8ab31d3) into master (d70140d) will not change coverage. The diff coverage is
n/a
.
@@ Coverage Diff @@
## master #216 +/- ##
=======================================
Coverage 68.40% 68.40%
=======================================
Files 78 78
Lines 14523 14523
=======================================
Hits 9934 9934
Misses 4589 4589
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update d70140d...8ab31d3. Read the comment docs.
@alicekwak, I have left some comments. Some need to be addressed before merging the pr, but some other ones are just some things for us to discuss later (e.g., Dane's paper heuristic for coreference). Let me know if you'd like to meet and discuss any of this before you make changes! Thank you! This looks good---only small changes needed!
@maxaalexeeva, thank you for reviewing all these! I'll make sure to address the issues found before merging this PR. Also, let's discuss some of the issues in our next meeting.
@alicekwak This is great code! It looks like Masha has already signed off on the rule changes. I have only one change request for your code and then I will approve the merge. I left a comment in the code for the change that I'd like you to address. Sorry for delaying this review!
@pauldhein No problem! Thanks so much for reviewing my PR. I'll let you know when I'm done changing the code as you suggested.
Thanks for fixing that @alicekwak. This PR is now approved and ready to merge as soon as you are ready.
Major changes
Minor change