Skip to content

Encode URL parameters - #117

Merged
faniereynders merged 1 commit into
RestCode:masterfrom
xps:master
Jun 13, 2017
Merged

faniereynders merged 1 commit into
RestCode:masterfrom
xps:master

Conversation

@xps

@xps xps commented Nov 1, 2016

Copy link
Copy Markdown
Contributor

This is a change so that URL parameters are properly encoded. Previously a call such as:

CreateItem("John&Johnson")

Would produce a request with the following URL:

.../create-item?name=John&Johnson

(i.e, the ampersand isn't encoded and the server only receives the first part of the string, "John", as the parameter value).

Strangely there was already a couple of EncodeParam() methods but they weren't called anywhere, so I just made sure they were applied. I am a bit puzzled as to how this issue could have stayed undetected so I might be missing something.

There was code to format dates with .ToString("o") so I simply updated the overload of EncodeParam() that deals with dates to use the same format.

The changes in the .cs file are auto-generated, the real changes are in the .tt file.

I am hoping that this can be merged, and that the NuGet packages can be updated soonish. Let me know if I can do anything else to assist.

@The-Futurist

Copy link
Copy Markdown

@xps - I see your pull request is rather old! Have you had challenges with thus repo accepting pull requests? The change in your PR, do you now just use your fork or did the same change somehow get made into the main repo by someone else?

@xps

xps commented Jun 11, 2017

Copy link
Copy Markdown
Contributor Author

@Korporal - I haven't heard from the maintainers after submitting my pull request, and there hasn't been any commits to the main branch since then either. At this point, I do use my own fork.

@The-Futurist

Copy link
Copy Markdown

@xps - I did communicate with the owner a little and it seems that his level of involvement or enthusiasm is pretty much what it is. This is a shame because it discourages others from submitting changes and eventually makes the repo less attractive because there's no real effort being put into improving the code.

I posted this question too:

#121

and haven't seen any reply so I'm now wondering whether I should embark on my own effort, one that I'd make an effort to maintain...

@faniereynders

faniereynders commented Jun 13, 2017 via email

Copy link
Copy Markdown
Member

@The-Futurist

Copy link
Copy Markdown

@faniereynders - Thanks for responding, but you wrote:

"but it still stays a community effort."

and people are submitting pull requests and issues that you don't have time to respond to. Yes it's a community effort but the community does have some reliance on the owner(s) to approve/deny pull requests!

I don't see how you can expect the "community" to do much when they must eventually wait for the owner(s) to respond to pull requests?

If you're too busy (which you clearly are) then I don't see how we can contribute much to your project, all people can do is submit pull requests and wait - or make their fork more active than your repo and so eventually take over the effort.

@faniereynders

Copy link
Copy Markdown
Member

To get back to this PR, I'm going to merge it back to master. Many thanks @xps! @Korporal let's continue this discussion on the Gitter space https://gitter.im/RestCode

@faniereynders
faniereynders merged commit 08f33eb into RestCode:master Jun 13, 2017
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