feat(generated)!: stop generating service names with duplicate Servic… #105
Conversation
arranf
force-pushed
the
feature/service-generation
branch
from
b3f0bee to
4bdf987
Compare
|
Awesome thanks for opening this. A few remaining steps: Could you run I think you'll need to update the examples. Could you also update the examples in the README.md? |
|
@tatethurston Will do and then I'll tag you again. |
| function getServiceName(name: string): string { | ||
| return name.toLowerCase().endsWith("service") | ||
| ? name.replace(new RegExp("(?:s)*service", "i"), "") | ||
| : name; | ||
| } | ||
|
|
There was a problem hiding this comment.
I think google-protobuf always appends Service -- did you check to see if that is the case?
If that is true, we can drop the regex and do name.slice(0, name.lastIndexOf('Service'))
There was a problem hiding this comment.
Ah I'm wrong about this -- I do add the service suffix. Instead of the above, we'll want to drop Service from here: https://github.com/tatethurston/TwirpScript/blob/main/src/autogenerate/index.ts#L421-L426
There was a problem hiding this comment.
Ah I'm wrong about this -- I do add the service suffix. Instead of the above, we'll want to drop
Servicefrom here: https://github.com/tatethurston/TwirpScript/blob/main/src/autogenerate/index.ts#L421-L426
Yeah I spotted this area of the codebase and originally I had this inlined in a few places in that file but it seemed silly to repeatedly run the regex. Eventually I figured that setting the name in a way so that we can assume safely that it doesn't have Service inside it would be best.
Open to more thoughts on this though - I'm not as familiar with the codebase as you are and didn't spend too long playing around. Was mostly trying to validate this was a sane idea.
There was a problem hiding this comment.
Let's delete the two references to Service from autogenerate so the service name ends up exactly as written in the generated code.
|
Implemented in #125 |
…e suffix
Fixes #104
Consider this PR a draft. I'll round out any edge cases.