Skip to content

Handling a different Map of parameters for forms in parseBody().#366

Closed
matthieu-labas wants to merge 4 commits intoNanoHttpd:masterfrom
matthieu-labas:master
Closed

Handling a different Map of parameters for forms in parseBody().#366
matthieu-labas wants to merge 4 commits intoNanoHttpd:masterfrom
matthieu-labas:master

Conversation

@matthieu-labas
Copy link
Copy Markdown
Contributor

Suggestion of correction for #365 .

@LordFokas
Copy link
Copy Markdown
Member

This will conflict with upcoming 3.0.0
Manually merging will be necessary, but at least we already have the logic.
Can you create a test that fails without your fix to ensure there is no regression ever?
Thank you for the help anyways :)

@matthieu-labas
Copy link
Copy Markdown
Contributor Author

Sure, I'll try that (good time to see how testing works ;)).

@LordFokas
Copy link
Copy Markdown
Member

The problem with all this is that test are terribly broken... the same is happening to my 3.0 branch, and I think it comes from a long time ago here.

Something tells me there's going to be a major pain involving fixing the tests. I can smell these things miles away.

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.04%) to 85.387% when pulling 3116209 on matthieu-labas:master into 3d31497 on NanoHttpd:master.

@LordFokas
Copy link
Copy Markdown
Member

@matthieu-labas the master branch now has the first half of v3.0 implemented, which makes this PR highly incompatible.

Would you like to redo this for the new code base (and get the chance to get aquainted with the new version) or should I manually apply your logic on top of my code?

@matthieu-labas
Copy link
Copy Markdown
Contributor Author

Thanks, I'll try to find time tomorrow. I'll keep you posted anyway if I can't. Getting to know the new thing always sounds good.

@matthieu-labas
Copy link
Copy Markdown
Contributor Author

I just did #372 and will correct Travis checks when they pop up ;)
I'll also check how to add a test on that.

@LordFokas
Copy link
Copy Markdown
Member

I'm going to kill this PR because it has serious conflicts and we now have a new one for the 3.0.0 code base.

@LordFokas LordFokas closed this Sep 12, 2016
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