Create MarkdownService, EmojisService, CodesOfConductService and MetaService #2937
Create MarkdownService, EmojisService, CodesOfConductService and MetaService #2937gmlewis merged 15 commits intogoogle:masterfrom
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2937 +/- ##
==========================================
- Coverage 98.17% 98.13% -0.05%
==========================================
Files 145 148 +3
Lines 12767 12783 +16
==========================================
+ Hits 12534 12544 +10
- Misses 158 164 +6
Partials 75 75
|
gmlewis
left a comment
There was a problem hiding this comment.
Thank you, @WillAbides !
I have some questions and a couple tweak requests, please.
Co-authored-by: Glenn Lewis <6598971+gmlewis@users.noreply.github.com>
|
@gmlewis I did the renaming you suggested and took it one further renaming I think you were right about renaming Markdown to Render. Even though the endpoint ends with /markdown` the description is "Render a Markdown document". |
gmlewis
left a comment
There was a problem hiding this comment.
Thank you, @WillAbides !
LGTM.
Awaiting second LGTM+Approval from any other contributor to this repo before merging.
|
One more possibility while we are breaking things. Would it make sense to rename |
I like it. Yes, please. |
gmlewis
left a comment
There was a problem hiding this comment.
I think these changes will make it easier to remove the deprecated functions in the future.
Thoughts?
|
Hey @gmlewis, I see that the PR is waiting on a few test function renames. Can I help in anyway to speed things up? I could raise a PR against P.S., can this comment be resolved? I gather that the required changes are done. |
Thanks, @abhijit-hota - marked comment as resolved. For the remaining items, I was interested in hearing what @WillAbides thinks about the ideas I toseed out, so let's please wait to hear. |
Co-authored-by: Glenn Lewis <6598971+gmlewis@users.noreply.github.com>
|
Sorry for the late response. I lost track of this one somehow. @abhijit-hota I appreciate your willingness to step in. @gmlewis Your suggestions make sense, so I added them. That reduced the code coverage a bit because the deprecated methods are no longer tested. IMO, there isn't a point in testing these anyway. |
gmlewis
left a comment
There was a problem hiding this comment.
Thank you, @WillAbides !
LGTM.
Awaiting second LGTM+Approval from any other contributor to this repo before merging.
|
LGTM. =) |
|
Thank you, @abhijit-hota ! |
closes #2936
This moves all Client methods from misc.go to new services.
In every case but one, the original Client method was left in place and refactored to call the new method. The exception is
Client.Markdownbecause the old method name is now the name of the service field.