-
Notifications
You must be signed in to change notification settings - Fork 179
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
Dialogue attribution and adverbs re: #106 #1080
base: main
Are you sure you want to change the base?
Conversation
@Serene-Arc I know this was a while ago, but are you still willing to work on it? Let me know. |
@Nytelife26 Hi! I am still willing to work on it :) This is a great tool |
I am still willing to work on it :)
Excellent! Thank you very much for your time and efforts. In that case, a few
things need to be done:
1. Can you rebase this PR over main, please?
2. Unit tests for this rule will also need to be written to ensure there
are as few false positives as possible
|
Sure, I'll get to it as soon as I can |
ef23c33
to
47a872b
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1080 +/- ##
==========================================
+ Coverage 90.19% 93.72% +3.53%
==========================================
Files 83 84 +1
Lines 1203 1275 +72
==========================================
+ Hits 1085 1195 +110
+ Misses 118 80 -38
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
I went ahead and applied those suggested changes for you, @Serene-Arc. Any chance you could create tests for this new check at some point, and perhaps rewrite the file docstrings to be more descriptive like in other checks? |
@Nytelife26 Ah, thanks! I'd already applied the suggested changes but I'll overwrite them with these. I've got some tests written, just been busy with some other stuff sorry, so I'll get them done soon and bring in the docstrings to be in line with the rest of the project. |
@Nytelife26 Ah, thanks! I'd already applied the suggested changes but
I'll overwrite them with these. I've got some tests written, just been
busy with some other stuff sorry, so I'll get them done soon and bring
in the docstrings to be in line with the rest of the project.
That's absolutely fine. Continue at your own pace - I just had some free
time and thought I'd give you a helping hand :)
Let me know if you need anything, and good luck with your other projects
(or whatever it is you've been busy with).
|
I implemented the rule from Steven King about adverbs in dialogue atribution, as described in issue #106
It's a fairly simple regex string that finds a piece of dialogue and checks to see if there is an adverb in the following phrase. I think it's fairly fragile but it's worked on the example texts that I've tested it with.
This is my first pull request and my first time contributing to a project that isn't my own so any help would be appreciated :)