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.

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

Enables custom attributes that can include content #89

Merged
merged 2 commits into from
Oct 31, 2018

Conversation

carl-berg
Copy link
Collaborator

@carl-berg carl-berg commented Oct 29, 2018

Somewhat relates to #48

Solves a case where we want to use a custom verb (PATCH), but also include content. Could you take a look @mattiasnordqvist and see if you agree or would like to solve this some other way?

This PR however breaks backwards compatability of HttpRequestFactory. It could be easily rewritten to not break compatability (by keeping ResolveHttpMethod and maybe adding a ResolveIncludeContent or something like that), but that seems slightly clumsier.

@carl-berg
Copy link
Collaborator Author

Thinking about it, i guess a "breaking" change in HttpRequestFactory is not really a breaking change to a consumer of the package since it is instantiated only once and that's in an internal class, so i would probably think this is a non breaking change.

I need this fix soon, so i'm gonna try to create a pre-release myself (not sure if i can but i'll give it a try).

@carl-berg carl-berg merged commit 5a7ba1e into master Oct 31, 2018
@carl-berg
Copy link
Collaborator Author

Nope, doesn't seem to be published automatically from the appveyor build, so i might need your help with that @mattiasnordqvist...

@carl-berg carl-berg deleted the content-inclusion-built-into-verb-attributes branch October 31, 2018 11:45
@mattiasnordqvist
Copy link
Owner

Sorry! vad har hänt här? :) Har inte loggat in på github på ett tag. Ring mig å berätta vetja!

@carl-berg
Copy link
Collaborator Author

Hehe, såg inte svaret förrän nu. Kort och gott ville vi få in funktionalitet för att göra en PATCH-request vilket gick utan modifikation, men behövde även kunna skicka med content vilket inte gick utan modifikation. Vill inte gärna ringa och störa, men slå du en signal när du har tid :-)

@mattiasnordqvist
Copy link
Owner

Det går inte att störa någon som inte är upptagen! :D Der är redan mergat eller? Vill du ha release å sånt?

@carl-berg
Copy link
Collaborator Author

Tack för release! Verkar funka bra!

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

Successfully merging this pull request may close these issues.

None yet

2 participants