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

WebServiceTarget - Enable OptimizeBufferReuse by default & Ability to add HTTP request headers #1912

Merged
merged 2 commits into from
Jun 23, 2017

Conversation

snakefoot
Copy link
Contributor

@snakefoot snakefoot commented Jan 13, 2017

Waiting for NLog ver. 4.5


This change is Reviewable

@snakefoot snakefoot force-pushed the WebServiceOptimizeBufferReuse branch from 07d4e23 to b6d67f7 Compare January 13, 2017 22:04
@codecov-io
Copy link

codecov-io commented Jan 13, 2017

Codecov Report

Merging #1912 into master will increase coverage by <1%.
The diff coverage is 77%.

@@           Coverage Diff           @@
##           master   #1912    +/-   ##
=======================================
+ Coverage      81%     81%   +<1%     
=======================================
  Files         288     288            
  Lines       19840   19881    +41     
  Branches     2339    2345     +6     
=======================================
+ Hits        16135   16185    +50     
- Misses       3109    3113     +4     
+ Partials      596     583    -13

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bc25298...9d047c2. Read the comment docs.

@snakefoot
Copy link
Contributor Author

Added support for HttpWebRequest Headers. Not sure if it should be pushed to NLog 5.0, as it is a breaking change.

@snakefoot snakefoot force-pushed the WebServiceOptimizeBufferReuse branch from b92f72f to 3de4cf6 Compare March 31, 2017 22:34
@304NotModified 304NotModified changed the title WebServiceTarget - Enable OptimizeBufferReuse by default WebServiceTarget - Enable OptimizeBufferReuse by default & Ability to add HTTP request headers Apr 3, 2017
@304NotModified
Copy link
Member

304NotModified commented Apr 3, 2017

Where is the breaking change?

Is it this one? https://github.com/NLog/NLog/pull/1912/files#diff-d37f2c9ae5632b4d37d050856c577959R220

I think it won't be a breaking change if we restore the body of DoInvoke(object[] parameters, AsyncContinuation continuation) (yes code duplication, but we could add a warning/task)

@snakefoot
Copy link
Contributor Author

snakefoot commented Apr 3, 2017

Where is the breaking change? Is it this one?

Yes. This method now throws an exception:

protected override void DoInvoke(object[] parameters, AsyncContinuation continuation)
{
   // method is not used, instead asynchronous overload will be used
   throw new NotImplementedException();
}

As it has been replaced by this override method:

protected override void DoInvoke(object[] parameters, AsyncLogEventInfo logEvent)

@snakefoot snakefoot force-pushed the WebServiceOptimizeBufferReuse branch from 3de4cf6 to 9d047c2 Compare April 4, 2017 16:31
@snakefoot
Copy link
Contributor Author

@304NotModified Now modified the code, so it is no longer a breaking change.

@304NotModified 304NotModified merged commit 53df971 into NLog:master Jun 23, 2017
@304NotModified
Copy link
Member

\0/ thanks!

@304NotModified 304NotModified modified the milestones: 4.5, 4.5 beta ? Oct 10, 2017
@snakefoot snakefoot deleted the WebServiceOptimizeBufferReuse branch October 10, 2017 20:39
@snakefoot snakefoot mentioned this pull request Nov 23, 2017
@snakefoot
Copy link
Contributor Author

Updated documentation:

https://github.com/NLog/NLog/wiki/WebService-target

@304NotModified 304NotModified added the documentation done all docs done (wiki, api docs, lists on nlog-project.org, xmldocs) label Nov 27, 2017
@snakefoot snakefoot modified the milestones: 4.5 beta ?, 4.5 Aug 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation done all docs done (wiki, api docs, lists on nlog-project.org, xmldocs) feature needs documentation on wiki performance webservice-target
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants