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.

Sign up for GitHub

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

Jump to bottom

Avoided url appending with slash when matrix parameter exists #999

Merged
merged 2 commits into from
Jul 18, 2019
Merged

Avoided url appending with slash when matrix parameter exists #999

merged 2 commits into from
Jul 18, 2019

Conversation

ranjitc5
Copy link
Contributor

@ranjitc5 ranjitc5 commented Jul 1, 2019

Fixed an issue of passing matrix parameter using RequestTemplate

More details at #998

@ranjitc5
Copy link
Contributor Author

ranjitc5 commented Jul 8, 2019

Can anyone please check it ?

velo
velo previously requested changes Jul 8, 2019
View reviewed changes
Copy link
Member

@velo velo left a comment

Choose a reason for hiding this comment

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

Could you also include a unit test? To prevent from breaking again in the future?

@kdavisk6
Copy link
Member

kdavisk6 commented Jul 8, 2019

I’m just back from vacation. I will review this later today also.

@ranjitc5 ranjitc5 closed this Jul 10, 2019
@ranjitc5 ranjitc5 reopened this Jul 10, 2019
@ranjitc5
Copy link
Contributor Author

Could you also include a unit test? To prevent from breaking again in the future?

I have added UT as well now.

kdavisk6
kdavisk6 approved these changes Jul 12, 2019
View reviewed changes
Copy link
Member

@kdavisk6 kdavisk6 left a comment

Choose a reason for hiding this comment

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

@ranjitc5

Thanks for your work. I've approved this PR and marked it as ready to merge. If there is no additional feedback from the other contributors in the next few days, this PR will be merged.

@kdavisk6 kdavisk6 dismissed velo’s stale review July 12, 2019 12:15

I've reviewed and verified that there is a unit test.

@kdavisk6 kdavisk6 added the ready to merge Will be merged if no other member ask for changes label Jul 12, 2019
@kdavisk6 kdavisk6 added this to the 10.2.4 milestone Jul 18, 2019
@kdavisk6 kdavisk6 merged commit 028ae86 into OpenFeign:master Jul 18, 2019
velo pushed a commit that referenced this pull request Oct 7, 2024
@ranjitc5 @kdavisk6
Avoided url appending with slash when matrix parameter exists (#999)
cab6eed 
* Avoided url appending with slash when matrix parameter exists

* Added UT to cover url appending with slash when matrix parameter exists
velo pushed a commit that referenced this pull request Oct 8, 2024
@ranjitc5 @kdavisk6
Avoided url appending with slash when matrix parameter exists (#999)
a57f890 
* Avoided url appending with slash when matrix parameter exists

* Added UT to cover url appending with slash when matrix parameter exists
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kdavisk6 kdavisk6 kdavisk6 approved these changes

@velo velo velo left review comments

Assignees
No one assigned
Labels
ready to merge Will be merged if no other member ask for changes
Projects
None yet
Milestone
10.3.0
Development

Successfully merging this pull request may close these issues.
3 participants
@ranjitc5 @kdavisk6 @velo