Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Refactor router for readability #1796

Merged
merged 2 commits into from
Mar 5, 2021

Conversation

aldas
Copy link
Contributor

@aldas aldas commented Mar 2, 2021

  • renamed variables for readability
  • refactored tests to table driver (this way it is easier to test each case with breakpoints)

2 commits to separate these changes in history

NB: I took several tests from my own implementation and some of them have FIXME notes. These are to document behaviors where I think currently we are not routing correctly.

For tests to highlight problematic param route + ending with slash - I will provide fix in separate PR as it is quite easy to fix.

@codecov
Copy link

codecov bot commented Mar 2, 2021

Codecov Report

Merging #1796 (eecd0e4) into master (6f9b71c) will increase coverage by 0.03%.
The diff coverage is 99.21%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1796      +/-   ##
==========================================
+ Coverage   89.56%   89.60%   +0.03%     
==========================================
  Files          32       32              
  Lines        2646     2656      +10     
==========================================
+ Hits         2370     2380      +10     
  Misses        178      178              
  Partials       98       98              
Impacted Files Coverage Δ
router.go 96.04% <99.21%> (+0.16%) ⬆️

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 6f9b71c...eecd0e4. Read the comment docs.

Copy link
Contributor

@lammel lammel left a comment

Choose a reason for hiding this comment

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

Finally readable router code. Awesome.

@lammel
Copy link
Contributor

lammel commented Mar 5, 2021

Great to see comments with diagrams to help understand the testcase.
Hopefully we won't have to change them anytime soon.

@lammel lammel merged commit 664cf8c into labstack:master Mar 5, 2021
This was referenced Mar 8, 2021
@aldas aldas deleted the refactor_router_for_readability branch May 23, 2021 06:07
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants