Skip to content

fix naming convention between access log format vs encoding#710

Merged
istio-testing merged 1 commit intoistio:masterfrom
GregHanson:access_log_encoding
Nov 19, 2018
Merged

fix naming convention between access log format vs encoding#710
istio-testing merged 1 commit intoistio:masterfrom
GregHanson:access_log_encoding

Conversation

@GregHanson
Copy link
Member

No description provided.

@googlebot googlebot added the cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. label Nov 19, 2018
@GregHanson
Copy link
Member Author

related to istio/istio#10056


// Encoding for the proxy access log (text or json).
// Default value is text.
string access_log_encoding = 27;
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be an enum?

Copy link
Member Author

Choose a reason for hiding this comment

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

@mandarjog @rshriram care to weigh in?

Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense for this to be enum.
The other PR probably is merged by now, so needs the same fix ...

Copy link
Member Author

Choose a reason for hiding this comment

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

done

@GregHanson GregHanson changed the title fix naming convention between access log format vs encoding [WIP] fix naming convention between access log format vs encoding Nov 19, 2018
@istio-testing istio-testing added the do-not-merge/work-in-progress Block merging of a PR because it isn't ready yet. label Nov 19, 2018
@GregHanson GregHanson changed the title [WIP] fix naming convention between access log format vs encoding fix naming convention between access log format vs encoding Nov 19, 2018
@istio-testing istio-testing removed the do-not-merge/work-in-progress Block merging of a PR because it isn't ready yet. label Nov 19, 2018
@mandarjog
Copy link
Contributor

/lgtm

@istio-testing
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: GregHanson, mandarjog

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@istio-testing istio-testing merged commit 62032e0 into istio:master Nov 19, 2018
@GregHanson GregHanson deleted the access_log_encoding branch November 20, 2018 14:06
louiscryan pushed a commit to louiscryan/api that referenced this pull request Jan 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants