-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
chore(endpoints): rename region tags at endpoints/getting-started/k8s/esp_echo_http.yaml #13009
base: main
Are you sure you want to change the base?
Conversation
…/esp_echo_http.yaml
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi team,
I'm currently reviewing this pull request and will have a full review up in a few minutes. In the meantime, here's a quick summary to get everyone up to speed:
This pull request, submitted by eapl-gemugami, renames region tags in the esp_echo_http.yaml
file within the endpoints/getting-started/k8s
directory. The intent is to fix several internal bugs (b/347825510, b/384989786, b/347825503) related to region tagging within the Kubernetes deployment configuration file.
Specifically, the changes involve renaming the region tags in the esp_echo_http.yaml
file. The changes are as follows:
[START secret-1]
and[END secret-1]
are renamed to[START endpoints_secret-1_yaml_python]
and[END endpoints_secret-1_yaml_python]
respectively (lines 41 and 46).[START service]
and[END service]
are renamed to[START endpoints_service_yaml_python]
and[END endpoints_service_yaml_python]
respectively (lines 47 and 58).[START secret-2]
and[END secret-2]
are renamed to[START endpoints_secret-2_yaml_python]
and[END endpoints_secret-2_yaml_python]
respectively (lines 61 and 66).
These changes ensure consistency and clarity in the documentation's region tagging, addressing the reported bugs.
A little haiku to lighten the mood:
Code flows like a stream,
Tags renamed, bugs now all fixed,
Clean code, joy abounds.
Thanks,
Gemini
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This pull request renames region tags in endpoints/getting-started/k8s/esp_echo_http.yaml
. The changes are straightforward and improve the consistency of region tag naming by following a more descriptive and informative naming convention. I appreciate that the author has followed the checklist and ensured the lint checks pass. I didn't find any explicit style guide provided, so I'm defaulting to commonly used style guides per the instructions.
# [START endpoints_secret-1_yaml_python] | ||
volumes: | ||
- name: service-account-creds | ||
secret: | ||
secretName: service-account-creds | ||
# [END secret-1] | ||
# [START service] | ||
# [END endpoints_secret-1_yaml_python] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new region tag names are much more descriptive and helpful. By including the product area (endpoints
), the snippet number (1
), the file type (yaml
), and the language (python
), the new names clearly communicate the purpose of each code snippet. This is good practice for maintainability and follows the recommendations in the AUTHORING_GUIDE.md
document. It also makes it easier to distinguish between different code snippets when they are referenced in documentation or other resources.
# [START endpoints_secret-1_yaml_python] | |
volumes: | |
- name: service-account-creds | |
secret: | |
secretName: service-account-creds | |
# [END secret-1] | |
# [START service] | |
# [END endpoints_secret-1_yaml_python] | |
# [START endpoints_secret-1_yaml_python] | |
volumes: | |
- name: service-account-creds | |
secret: | |
secretName: service-account-creds | |
# [END endpoints_secret-1_yaml_python] |
# [START endpoints_secret-2_yaml_python] | ||
volumeMounts: | ||
- mountPath: /etc/nginx/creds | ||
name: service-account-creds | ||
readOnly: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change to the secret-2
region tag is consistent with the other changes in this pull request and improves the clarity of the code. By using a consistent naming scheme, it's easier to understand the purpose of each region tag and how it relates to the overall structure of the application. This also simplifies the process of updating or modifying the region tags in the future, as there is a clear pattern to follow.
# [START endpoints_secret-2_yaml_python] | |
volumeMounts: | |
- mountPath: /etc/nginx/creds | |
name: service-account-creds | |
readOnly: true | |
# [START endpoints_secret-2_yaml_python] | |
volumeMounts: | |
- mountPath: /etc/nginx/creds | |
name: service-account-creds | |
readOnly: true | |
# [END endpoints_secret-2_yaml_python] |
Here is the summary of changes. You are about to add 3 region tags.
You are about to delete 3 region tags.
This comment is generated by snippet-bot.
|
Description
Fixes Internal:
b/347825510
b/384989786
b/347825503
Checklist
nox -s lint
(see Test Environment Setup)