- Notifications
You must be signed in to change notification settings - Fork2.9k
feat(appengine): quickstart version#10010
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
feat(appengine): quickstart version#10010
Uh oh!
There was an error while loading.Please reload this page.
Conversation
snippet-botbot commentedFeb 3, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Here is the summary of changes. You are about to add 1 region tag.
This comment is generated bysnippet-bot.
|
ghost left a comment
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 provide my feedback shortly. In the meantime, here's a summary of the changes for you and other reviewers to get up to speed quickly:
This pull request updates the Java version in the App Engine Qwik Start sample to Java 21. The changes are focused on theappengine-java21/helloworld/http-server
directory.
Intent:
The primary goal is to modernize the sample application by using the latest Java version, improving compatibility and potentially leveraging new features.
Changes:
- The
app.yaml
file was updated to specifyruntime: java21
. (Line 16 ofappengine-java21/helloworld/http-server/src/main/appengine/app.yaml
) - A new
README.md
file was added to provide instructions for deploying the updated application. (File:appengine-java21/helloworld/http-server/README.md
) - The
pom.xml
file was updated to reflect the Java 21 runtime. (File:appengine-java21/helloworld/http-server/pom.xml
) - The main application file (
Main.java
) remains largely unchanged, demonstrating a simple HTTP server. (File:appengine-java21/helloworld/http-server/src/main/java/com/example/appengine/Main.java
)
Activity:
The pull request references Buganizer issuehttp://b/393352626, suggesting this is part of a larger effort to update App Engine samples.
I'll provide a more detailed review shortly. Thanks!
And now, a little haiku to brighten your day:
Java twenty-one,
Code compiles, a clean deploy,
Happy coding now!
ghost left a comment
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 updates the Java version to Java 21 in the App Engine sample application. The changes include updatingapp.yaml
, adding apom.xml
for Maven build management, and creating aREADME.md
file with instructions. The implementation looks good overall, and the added documentation is helpful. Here are a few minor suggestions:
Summary of Google Java Style Guide elements referenced:
- Comments: Documented using Javadoc style.
- Naming: Used descriptive names for classes and variables.
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.
issue(blocking): remove this file. All usage documentation should be provided on cloud.google.com.
Hello@manishchaudhari-ssk -- it doesn't look like my comment has been addressed; I am going to switch this to a draft PR. |
Uh oh!
There was an error while loading.Please reload this page.