Rename enableJSONPOST #442

Closed
seancorfield opened this Issue Jul 11, 2016 · 15 comments

Comments

Projects
None yet
7 participants
@seancorfield
Member

seancorfield commented Jul 11, 2016

This is poorly named since it enables more than just JSON support and it does it for all HTTP verbs, not just POST. A better name would be enableRESTbody but I'd like feedback from the community on this.

Note that it will be a breaking change for anyone using enableJSONPOST so that option needs to throw an exception at startup to avoid silent changes of behavior.

@seancorfield seancorfield added this to the 4.0 milestone Jul 11, 2016

@seancorfield seancorfield self-assigned this Jul 11, 2016

@Richardtugwell

This comment has been minimized.

Show comment
Hide comment
@Richardtugwell

Richardtugwell Jul 11, 2016

I'm using this to POST data that isn't strictly REST-ful - e.g objects that
could encapsulate form data + other stuff for example. So I'm not sure
about having REST in the name...

On 11 July 2016 at 20:58, Sean Corfield notifications@github.com wrote:

This is poorly named since it enables more than just JSON support and it
does it for all HTTP verbs, not just POST. A better name would be
enableRESTbody but I'd like feedback from the community on this.

Note that it will be a breaking change for anyone using enableJSONPOST
so that option needs to throw an exception at startup to avoid silent
changes of behavior.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#442, or mute the thread
https://github.com/notifications/unsubscribe/ABJDLV9b8Ky-keVgsWntDuw4OZxkliI3ks5qUqBqgaJpZM4JJuSH
.

Richard Tugwell
http://blog.richardtugwell.com
r.tugwell@forthmedia.com

I'm using this to POST data that isn't strictly REST-ful - e.g objects that
could encapsulate form data + other stuff for example. So I'm not sure
about having REST in the name...

On 11 July 2016 at 20:58, Sean Corfield notifications@github.com wrote:

This is poorly named since it enables more than just JSON support and it
does it for all HTTP verbs, not just POST. A better name would be
enableRESTbody but I'd like feedback from the community on this.

Note that it will be a breaking change for anyone using enableJSONPOST
so that option needs to throw an exception at startup to avoid silent
changes of behavior.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#442, or mute the thread
https://github.com/notifications/unsubscribe/ABJDLV9b8Ky-keVgsWntDuw4OZxkliI3ks5qUqBqgaJpZM4JJuSH
.

Richard Tugwell
http://blog.richardtugwell.com
r.tugwell@forthmedia.com

@mjhagen

This comment has been minimized.

Show comment
Hide comment
@mjhagen

mjhagen Jul 11, 2016

Contributor

Is enableHTTPbody better?

Contributor

mjhagen commented Jul 11, 2016

Is enableHTTPbody better?

@Richardtugwell

This comment has been minimized.

Show comment
Hide comment
@Richardtugwell

Richardtugwell Jul 11, 2016

Naming is really tough....

On 11 July 2016 at 21:26, Mingo Hagen notifications@github.com wrote:

Is enableHTTPbody better?


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#442 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/ABJDLXaETSefiysOOtDfhMcoWjIAL_O3ks5qUqcSgaJpZM4JJuSH
.

Richard Tugwell
http://blog.richardtugwell.com
r.tugwell@forthmedia.com

Naming is really tough....

On 11 July 2016 at 21:26, Mingo Hagen notifications@github.com wrote:

Is enableHTTPbody better?


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#442 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/ABJDLXaETSefiysOOtDfhMcoWjIAL_O3ks5qUqcSgaJpZM4JJuSH
.

Richard Tugwell
http://blog.richardtugwell.com
r.tugwell@forthmedia.com

@seancorfield

This comment has been minimized.

Show comment
Hide comment
@seancorfield

seancorfield Jul 11, 2016

Member

enableEncodedBody perhaps? Or enableBodyDecoding?

👍 on naming being really tough!

Member

seancorfield commented Jul 11, 2016

enableEncodedBody perhaps? Or enableBodyDecoding?

👍 on naming being really tough!

@jcberquist

This comment has been minimized.

Show comment
Hide comment
@jcberquist

jcberquist Jul 12, 2016

Contributor

To offer another opinion here - I like having decode in the name. Both enableHTTPBodyDecode and enableHTTPBodyDecoding sound good to me, though perhaps they are a bit long. @seancorfield's simpler/shorter suggestion of enableBodyDecoding sounds good too.

Contributor

jcberquist commented Jul 12, 2016

To offer another opinion here - I like having decode in the name. Both enableHTTPBodyDecode and enableHTTPBodyDecoding sound good to me, though perhaps they are a bit long. @seancorfield's simpler/shorter suggestion of enableBodyDecoding sounds good too.

@Richardtugwell

This comment has been minimized.

Show comment
Hide comment
@Richardtugwell

Richardtugwell Jul 12, 2016

It's just personal but I prefer 'doSomething = true|false' rather
than 'enableDoSomething = true|false'

So maybe parseRequestBody?

On 12 July 2016 at 01:16, jcberquist notifications@github.com wrote:

To offer another opinion here - I like having decode in the name. Both
enableHTTPBodyDecode and enableHTTPBodyDecoding sound good to me, though
perhaps they are a bit long. @seancorfield
https://github.com/seancorfield's simpler/shorter suggestion of
enableBodyDecoding sounds good too.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#442 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/ABJDLTIaeesLDDpx80OvFPyNRr9szY2tks5qUtzmgaJpZM4JJuSH
.

Richard Tugwell
http://blog.richardtugwell.com
r.tugwell@forthmedia.com

It's just personal but I prefer 'doSomething = true|false' rather
than 'enableDoSomething = true|false'

So maybe parseRequestBody?

On 12 July 2016 at 01:16, jcberquist notifications@github.com wrote:

To offer another opinion here - I like having decode in the name. Both
enableHTTPBodyDecode and enableHTTPBodyDecoding sound good to me, though
perhaps they are a bit long. @seancorfield
https://github.com/seancorfield's simpler/shorter suggestion of
enableBodyDecoding sounds good too.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#442 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/ABJDLTIaeesLDDpx80OvFPyNRr9szY2tks5qUtzmgaJpZM4JJuSH
.

Richard Tugwell
http://blog.richardtugwell.com
r.tugwell@forthmedia.com

@mjhagen

This comment has been minimized.

Show comment
Hide comment
@mjhagen

mjhagen Jul 12, 2016

Contributor

Wouldn't that be decodeRequestBody then @Richardtugwell? Though parsing is broader than decoding I guess.

Contributor

mjhagen commented Jul 12, 2016

Wouldn't that be decodeRequestBody then @Richardtugwell? Though parsing is broader than decoding I guess.

@Richardtugwell

This comment has been minimized.

Show comment
Hide comment
@Richardtugwell

Richardtugwell Jul 12, 2016

Yeah - I'd be happy with that. Parse is a bit vague

On 12 July 2016 at 10:56, Mingo Hagen notifications@github.com wrote:

Wouldn't that be decodeRequestBody then @Richardtugwell
https://github.com/Richardtugwell? Though parsing is broader than
decoding I guess.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#442 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/ABJDLfXepiIk0cSwZQQxxZYootXkib80ks5qU2S8gaJpZM4JJuSH
.

Richard Tugwell
http://blog.richardtugwell.com
r.tugwell@forthmedia.com

Yeah - I'd be happy with that. Parse is a bit vague

On 12 July 2016 at 10:56, Mingo Hagen notifications@github.com wrote:

Wouldn't that be decodeRequestBody then @Richardtugwell
https://github.com/Richardtugwell? Though parsing is broader than
decoding I guess.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#442 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/ABJDLfXepiIk0cSwZQQxxZYootXkib80ks5qU2S8gaJpZM4JJuSH
.

Richard Tugwell
http://blog.richardtugwell.com
r.tugwell@forthmedia.com

@seancorfield

This comment has been minimized.

Show comment
Hide comment
@seancorfield

seancorfield Jul 12, 2016

Member

FWIW, FW/1 has used enableXyz settings in the past, although it has also used suppressAbc (but usually in transition as features are deprecated and phased out). I'd be happy with decodeRequestBody tho'.

Member

seancorfield commented Jul 12, 2016

FWIW, FW/1 has used enableXyz settings in the past, although it has also used suppressAbc (but usually in transition as features are deprecated and phased out). I'd be happy with decodeRequestBody tho'.

@jcberquist

This comment has been minimized.

Show comment
Hide comment
@jcberquist

jcberquist Jul 12, 2016

Contributor

decodeRequestBody sounds good to me, too.

Contributor

jcberquist commented Jul 12, 2016

decodeRequestBody sounds good to me, too.

@tonyjunkes

This comment has been minimized.

Show comment
Hide comment
@tonyjunkes

tonyjunkes Jul 12, 2016

Member

I'm for decodeRequestBody 👌🏻

Member

tonyjunkes commented Jul 12, 2016

I'm for decodeRequestBody 👌🏻

@seancorfield seancorfield added the ready label Jul 12, 2016

@cfvonner

This comment has been minimized.

Show comment
Hide comment
@cfvonner

cfvonner Jul 12, 2016

Contributor

decodeRequestBody works for me.

Contributor

cfvonner commented Jul 12, 2016

decodeRequestBody works for me.

@ddspringle

This comment has been minimized.

Show comment
Hide comment
@ddspringle

ddspringle Jul 12, 2016

+1 for decodeRequestBody

+1 for decodeRequestBody

@seancorfield

This comment has been minimized.

Show comment
Hide comment
@seancorfield

seancorfield Jul 12, 2016

Member

OK, decodeRequestBody it is!

Member

seancorfield commented Jul 12, 2016

OK, decodeRequestBody it is!

@seancorfield

This comment has been minimized.

Show comment
Hide comment
@seancorfield

seancorfield Jul 12, 2016

Member

This has been applied to the develop branch. Given the "breaking" nature of this, I think I'm going to cut Beta 2 and update the documentation.

Member

seancorfield commented Jul 12, 2016

This has been applied to the develop branch. Given the "breaking" nature of this, I think I'm going to cut Beta 2 and update the documentation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment