-
Notifications
You must be signed in to change notification settings - Fork 48
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
reduce the amount of spelling issues / WORDLIST #685
Conversation
Signed-off-by: Marcin <133694481+m7pr@users.noreply.github.com>
Hey @shajoezhu - I am ready with my part. I suggest we merge now, so everything that is required by #678 will already have spell-check fixed and you will only need to figure out the new spelling issues that will be triggered by GitHub Actions spelling check. I reckon it will be easier to solve future spelling issues one-by-one on a single PR than once again once multiple PRs are merged. |
Thanks, Marcin to work on this. I think we need to put it on hold merging it after #665. What you think @shajoezhu @gmbecker |
IMHO the sooner you merge, the sooner you start paying attention to spelling anywhere |
Signed-off-by: Marcin <133694481+m7pr@users.noreply.github.com>
Signed-off-by: Marcin <133694481+m7pr@users.noreply.github.com>
Signed-off-by: Marcin <133694481+m7pr@users.noreply.github.com>
Trust me, we did know very well about the problem which predates the current SME team. Anyway, if you insist we can merge it in already, but I have the feeling that this is going to break all PRs currently open and it is not a great timing for this. In general, we need to collaborate, and please at least warn us about it beforehand. Just let us know before starting working on these kinds of issues as it may not be the best timing for those. Also, this work is not on Kendis, and it is clearly something that has dependencies on our team. |
But this is my council: flag this on @gogonzo to liaison with @shajoezhu on the best way forward before any merge is being considered. In the interim, hold fire on this one. |
Apologies guys. The whole blame is on me. Bad planning. I chatted with Marcin and I ask him to complete that for rtables repo. Anyway, @shajoezhu please make a call what to do with this PR. Even if it will wait - I don't expect a lot of complex git conflicts to be resolved. |
hey guys, thanks for working on this. I am blocking the merge for now. I would like to resolve other issues first, which are priorities for the release |
Apologies, I was not aware that there is release going on. Take your time to merge it. Flag me also when you think it can be merged and I'll fix conflicts and update files after you made changes to your PRs. |
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.
hi @m7pr , can you add a NEWS entry please. Thanks!
Yes @shajoezhu - I did fix conflicts and updated NEWS with a relevant note about the spelling revision |
Signed-off-by: Joe Zhu <joe.zhu@roche.com>
Code Coverage Summary
Diff against main
Results for commit: 04124d4 Minimum allowed coverage is ♻️ This comment has been updated with latest results |
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! many thanks @m7pr
A follow-up after https://github.com/insightsengineering/coredev-tasks/issues/235
I am basically trying to catch all spelling issues, and try to put \code{} around parameter names, function names, package names, object names and data structure names.
Final result of
spelling::spell_check_package()