twitter / finatra

Fast, testable, Scala services built on TwitterServer and Finagle
https://twitter.github.io/finatra/
Apache License 2.0
2.27k stars 406 forks source link

finatra-http Issue#520 - PrefixedDSL support empty route #521

Closed acheek24 closed 4 years ago

acheek24 commented 4 years ago

Problem Route is currently always required, even when a prefix is supplied and no additional route is desirable.

Solution Default routes to an empty String and alter the existing validation. Prefix is now applied before validation.

claassistantio commented 4 years ago

CLA assistant check
All committers have signed the CLA.

codecov[bot] commented 4 years ago

Codecov Report

Merging #521 into develop will increase coverage by 0.00%. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop     #521   +/-   ##
========================================
  Coverage    91.38%   91.38%           
========================================
  Files          265      265           
  Lines         4215     4216    +1     
  Branches       302      308    +6     
========================================
+ Hits          3852     3853    +1     
  Misses         363      363           

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 d8d4d5d...96afcb3. Read the comment docs.

cacoco commented 4 years ago

@acheek24 thanks for the contribution! At first glance, this is not something we feel that we want to do or enable. The concept and definition of a "prefix" is that it is always the first segment of a Route and not a Route itself. If you have "no other route desired" then the appropriate thing to do is to simply define the routes (it is fairly straightforward to also define a constant that you can re-use as the first segment if desired).

If you could provide more context as to the actual problem (with examples) that is being solved by this change, it may help but as it stands this is not something we feel should change in this manner.

Thanks!