Skip to content

Conversation

@etr
Copy link
Owner

@etr etr commented Feb 29, 2020

Identify the Bug

As by discussion in this PR: #180 . The http_response object contains protected members for no apparent reason. This, besides making the internals of the class visible from the outside, impedes improvement to the code.

Description of the Change

Move the members to the private section.
There wasn't really a good reason for them to be protected in the first place. It was just a residual from before the Jan 2019 cleanup to the http_response chain of dependencies.

Possible Drawbacks

If external code has taken a dependency on these fields, this change would result in a failure for such code.

Verification Process

unit/integration testing. The change will also run through Travis before being pushed.

Release Notes

All protected class members variables in http_response have been changed to be private.

They never should have been protected in the first place, as they
are accessible through class methods.
@etr etr self-assigned this Feb 29, 2020
@codecov-io
Copy link

Codecov Report

Merging #181 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #181   +/-   ##
=======================================
  Coverage   95.19%   95.19%           
=======================================
  Files          35       35           
  Lines        3140     3140           
=======================================
  Hits         2989     2989           
  Misses        151      151
Impacted Files Coverage Δ
src/httpserver/http_response.hpp 80% <ø> (ø) ⬆️

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 9cdc901...9eef6d2. Read the comment docs.

@etr etr merged commit 04a23c8 into master Mar 1, 2020
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