-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Update folder structure for flex java 8 #7741
Conversation
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.
LGTM. I verified that all of the changes are under flexible/java-8. https://screenshot.googleplex.com/AhGRVYsVRPkXUAa. There's no feasible way for me to check if they are identical to the existing samples. I would assume you copied them by cp
. To be super cautious, you could compare the directory size or number of files/words between the old and new copy.
I used diff to check for the differences: https://screenshot.googleplex.com/7ux9TwYRfFkGySj |
Great! Thanks for sharing the diff result. |
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.
For problematic region tags, if used in the docs please add the gae_flex_
prefix or remove them completely:
There are 7 possible violations for not having product prefix.
flexible/java-8/errorreporting/src/main/java/com/example/flexible/errorreporting/ErrorReportingExample.java:34, tag flex_error_reporting
flexible/java-8/helloworld/build.gradle:14, tag gradle
flexible/java-8/helloworld/build.gradle:43, tag gretty
flexible/java-8/helloworld/build.gradle:49, tag model
flexible/java-8/memcache/pom.xml:52, tag dependencies
flexible/java-8/memcache/src/main/appengine/app.yaml:8, tag config
flexible/java-8/memcache/src/main/java/com/example/memcache/MemcacheServlet.java:32, tag example
# limitations under the License. | ||
|
||
# Fail on non-zero return and print command to stdout | ||
set -xe |
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.
Is this file needed?
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.
I have just copied over the existing projects from the flexible/ folder to flexible/java-8 and this file was already there as a part of #538. I think it is okay to leave it as is since this repository is for samples. wdyt?
I would suggest you double check or at least notify the Docs team before changing the region tags. They might be used by the docs to locate the code blocks. |
Since this is a copy to a new directory, this shouldn't be an issue until you delete the old duplicates. |
We need to update the structure of flexible folder to reflect apps as per the java version. We need to copy the java8 apps under flexible/java-8.
P.S. Not deleting the files under flexible/ right now as we need to update the documentation to use the flexible/java-8/ apps. There will be a followup PR for cleanup.