Skip to content
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

Move Shower class to framework def instead of server/client def #689

Merged
merged 1 commit into from
Jun 17, 2020

Conversation

kelnos
Copy link
Member

@kelnos kelnos commented Jun 17, 2020

When generating models only, the framework definitions of the server and client are not rendered. However, the Shower class is still required for models, so move it into the framework support definitions list.

Contributing to Twilio

All third-party contributors acknowledge that any contributions they provide will be made under the same open-source license that the open-source project is provided under.

  • I acknowledge that all my contributions will be made under the project's license.

When generating models only, the framework definitions of the server and
client are not rendered.  However, the Shower class is still required
for models, so move it into the framework support definitions list.
@kelnos kelnos requested a review from blast-hardcheese June 17, 2020 05:25
@codecov-commenter
Copy link

codecov-commenter commented Jun 17, 2020

Codecov Report

Merging #689 into master will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #689      +/-   ##
==========================================
+ Coverage   81.17%   81.20%   +0.02%     
==========================================
  Files          78       78              
  Lines        5195     5197       +2     
  Branches      129      139      +10     
==========================================
+ Hits         4217     4220       +3     
+ Misses        978      977       -1     
Impacted Files Coverage Δ
...nerators/Java/AsyncHttpClientClientGenerator.scala 94.39% <ø> (-0.01%) ⬇️
...il/generators/Java/DropwizardServerGenerator.scala 98.01% <ø> (-0.02%) ⬇️
...uardrail/generators/Java/DropwizardGenerator.scala 25.64% <100.00%> (+4.01%) ⬆️
...n/src/main/scala/com/twilio/guardrail/Common.scala 100.00% <0.00%> (+1.19%) ⬆️

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 637e7e9...fe8eec9. Read the comment docs.

Copy link
Member

@blast-hardcheese blast-hardcheese left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What changes are required for consumers? I'm not seeing any tests change here

@kelnos
Copy link
Member Author

kelnos commented Jun 17, 2020

No changes are required. All this does is remove the same something from both the client and server generator's support defs, and add it to the framework's support defs. The framework support defs are always rendered, regardless of whether it's a client or server, so there's no user-visible change (except that generating just models will now work, which did not work before).

@blast-hardcheese
Copy link
Member

Was wondering about package membership, but if these aren't static classes then ok

@kelnos
Copy link
Member Author

kelnos commented Jun 17, 2020

Ah, nah, they're toplevels, and the package ends up being the same.

@kelnos kelnos merged commit a6f6a11 into guardrail-dev:master Jun 17, 2020
@kelnos kelnos deleted the dw-move-shower-def branch June 17, 2020 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants