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.

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

Hello.war app for the Quick start guide #24889

Merged

Conversation

samsonsouley
Copy link
Contributor

@samsonsouley samsonsouley commented Apr 3, 2024

Fixes #24801

Create a new module for the hello project with jakarta following the same structure of the old hello project
Add the module to the project
@samsonsouley samsonsouley changed the title first commit Hello.war app for the Quick start guide Apr 3, 2024
@samsonsouley samsonsouley marked this pull request as draft April 3, 2024 15:07
@OndroMih
Copy link
Contributor

OndroMih commented May 4, 2024

Hello, @samsonsouley , can you sign the Eclipse Contributor Agreement according to these instructions?

Log into the Eclipse projects forge (you will need to create an account with the Eclipse Foundation if you have not already done so); click on "Eclipse Contributor Agreement"; and Complete the form. Be sure to use the same email address when you register for the account that you intend to use on Git commit records.

@samsonsouley
Copy link
Contributor Author

samsonsouley commented May 5, 2024

Hello, @samsonsouley , can you sign the Eclipse Contributor Agreement according to these instructions?

Log into the Eclipse projects forge (you will need to create an account with the Eclipse Foundation if you have not already done so); click on "Eclipse Contributor Agreement"; and Complete the form. Be sure to use the same email address when you register for the account that you intend to use on Git commit records.

Hi Ondro,
Thanks it is done

@samsonsouley samsonsouley reopened this May 5, 2024
@OndroMih
Copy link
Contributor

OndroMih commented May 5, 2024

Great, taht was really quick, @samsonsouley !

Now we need to fix one other formal thing - we need copyright headers in each file. So, please, into each new file, add the following text at the top of the file:

/*
 * Copyright (c) 2024 Contributors to the Eclipse Foundation
 *
 * This program and the accompanying materials are made available under the
 * terms of the Eclipse Public License v. 2.0, which is available at
 * http://www.eclipse.org/legal/epl-2.0.
 *
 * This Source Code may also be made available under the following Secondary
 * Licenses when the conditions for such availability set forth in the
 * Eclipse Public License v. 2.0 are satisfied: GNU General Public License,
 * version 2 with the GNU Classpath Exception, which is available at
 * https://www.gnu.org/software/classpath/license.html.
 *
 * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
 */

It's a comment, so in XHTML and XML files, you'll need to replace /* and */ with XML comments= marks <!-- and -->. And in JSP it will be <%-- and --%>

If you only updated a file, just check that the existing header contains a line with Copyright (c) Contributors to the Eclipse Foundation - and check that the line contains year 2024.

@samsonsouley
Copy link
Contributor Author

Great, taht was really quick, @samsonsouley !

Now we need to fix one other formal thing - we need copyright headers in each file. So, please, into each new file, add the following text at the top of the file:

/*
 * Copyright (c) 2024 Contributors to the Eclipse Foundation
 *
 * This program and the accompanying materials are made available under the
 * terms of the Eclipse Public License v. 2.0, which is available at
 * http://www.eclipse.org/legal/epl-2.0.
 *
 * This Source Code may also be made available under the following Secondary
 * Licenses when the conditions for such availability set forth in the
 * Eclipse Public License v. 2.0 are satisfied: GNU General Public License,
 * version 2 with the GNU Classpath Exception, which is available at
 * https://www.gnu.org/software/classpath/license.html.
 *
 * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
 */

It's a comment, so in XHTML and XML files, you'll need to replace /* and */ with XML comments= marks <!-- and -->. And in JSP it will be <%-- and --%>

If you only updated a file, just check that the existing header contains a line with Copyright (c) Contributors to the Eclipse Foundation - and check that the line contains year 2024.

It is done

Comment on lines 73 to 97
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-dependency-plugin</artifactId>
<version>2.6</version>
<executions>
<execution>
<phase>validate</phase>
<goals>
<goal>copy</goal>
</goals>
<configuration>
<outputDirectory>${endorsed.dir}</outputDirectory>
<silent>true</silent>
<artifactItems>
<artifactItem>
<groupId>jakarta.platform</groupId>
<artifactId>jakarta.jakartaee-api</artifactId>
<version>${jakartaee}</version>
<type>jar</type>
</artifactItem>
</artifactItems>
</configuration>
</execution>
</executions>
</plugin>
Copy link
Contributor

@OndroMih OndroMih May 7, 2024

Choose a reason for hiding this comment

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

This whole section is completely useless, Java doesn't support endorsed libraries anymore and all works out of the box even without this. I think this was generated by your IDE. Please remove it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

docs/pom.xml Outdated
@@ -61,5 +61,6 @@
<module>troubleshooting-guide</module>
<module>upgrade-guide</module>
<module>distribution</module>
<module>EclipseGlassFishHelloSample</module>
Copy link
Contributor

Choose a reason for hiding this comment

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

Please rename the module so that it's similar to names of other modules. For example, this would fit more: eclipse-glassfish-hello-sample-app, or simply sample-app

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I took sample-app

@OndroMih
Copy link
Contributor

OndroMih commented May 7, 2024

Good job, @samsonsouley !

I left a few more comments to improve this, but when those are addressed, it's all good to go and you can turn this Draft into a normal pull request.

@OndroMih
Copy link
Contributor

OndroMih commented May 7, 2024

All looks good now, @samsonsouley .

You can remove the draft status now, so that committers can approve and merge this

docs/sample-app/pom.xml Outdated Show resolved Hide resolved
docs/sample-app/pom.xml Outdated Show resolved Hide resolved
docs/sample-app/pom.xml Outdated Show resolved Hide resolved
docs/sample-app/pom.xml Outdated Show resolved Hide resolved
<properties>
<maven.compiler.release>17</maven.compiler.release>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<failOnMissingWebXml>false</failOnMissingWebXml>
Copy link
Contributor

Choose a reason for hiding this comment

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

This line can also be removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Comment on lines 44 to 48
<dependency>
<groupId>org.glassfish.web</groupId>
<artifactId>jakarta.servlet.jsp.jstl</artifactId>
<version>3.0.1</version>
</dependency>
Copy link
Contributor

Choose a reason for hiding this comment

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

This can be removed, it's not used. JSP is already provided by the jakarta.jakartaee-api dependency

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well noted. It is done

docs/publish/pom.xml Outdated Show resolved Hide resolved
@samsonsouley samsonsouley marked this pull request as ready for review May 8, 2024 15:50
@arjantijms
Copy link
Contributor

Thanks! :)

@arjantijms arjantijms merged commit fffe06e into eclipse-ee4j:master May 25, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

doc: broken link to hello.war
4 participants