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.

Sign up for GitHub

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

Jump to bottom

TS-4771: Remove config, stats, and docs for ICP #943

Merged
merged 1 commit into from
Sep 7, 2016
Merged

TS-4771: Remove config, stats, and docs for ICP #943

merged 1 commit into from
Sep 7, 2016

Conversation

bryancall
Copy link
Contributor

No description provided.

@atsci
Copy link

atsci commented Aug 30, 2016

FreeBSD build successful! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/643/ for details.

@atsci
Copy link

atsci commented Aug 30, 2016

Linux build successful! See https://ci.trafficserver.apache.org/job/Github-Linux/539/ for details.

@atsci
Copy link

atsci commented Aug 30, 2016

FreeBSD build successful! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/644/ for details.

@atsci
Copy link

atsci commented Aug 30, 2016

Linux build successful! See https://ci.trafficserver.apache.org/job/Github-Linux/540/ for details.

@zwoop zwoop added this to the 7.0.0 milestone Aug 31, 2016
@zwoop zwoop self-assigned this Aug 31, 2016
zwoop
zwoop reviewed Aug 31, 2016
View reviewed changes
mgmt/RecordsConfig.cc
@@ -1299,6 +1299,9 @@ static const RecordElement RecordsConfig[] =
,
{RECT_CONFIG, "proxy.config.ssl.wire_trace_server_name", RECD_STRING, NULL , RECU_DYNAMIC, RR_NULL, RECC_STR, ".*", RECA_NULL}
,
{RECT_CONFIG, "proxy.config.ssl.cert.load_elevated", RECD_INT, "0", RECU_RESTART_TS, RR_NULL, RECC_INT, "[0-1]", RECA_READ_ONLY}
,

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this really be in this patch set ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, never mind, it got moved up.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it looked out of place.

@zwoop
Copy link
Contributor

zwoop commented Sep 1, 2016

Couple of minor question / issues:

  1. There are some references to the ICP Via headers in the cmd/traffic_via/tests directory. They should go away maybe? Or do we still generate this ?

  2. This doesn't remove the actual code as far as I can tell, is there another PR for that?

  3. I found one line still using a (now missing) ICP define:

example/redirect-1/redirect-1.c: else if (0 == strncmp(txn_method, TS_HTTP_METHOD_ICP_QUERY, length))

@bryancall
Copy link
Contributor Author

bryancall commented Sep 7, 2016 •
edited

This pull request disables ICP, removes the configuration, APIs, stats, and docs. I am breaking up removing ICP into pieces. Being that it is late in the release cycle it might be best to rip out the ICP code for the 7.1.0 release.

@zwoop
Copy link
Contributor

zwoop commented Sep 7, 2016

👍

@bryancall bryancall merged commit 22f6e59 into apache:master Sep 7, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@zwoop zwoop

Labels
Configuration Documentation Metrics
Projects
None yet
Milestone
7.0.0
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@bryancall @atsci @zwoop