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

Add all positive cases for TomEE #303

Closed
wants to merge 23 commits into from

Conversation

jannahsherif
Copy link
Contributor

No description provided.

@eclipse-starter-bot
Copy link

Can one of the admins verify this patch?

Copy link
Contributor

@m-reza-rahman m-reza-rahman left a comment

Choose a reason for hiding this comment

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

Please address review feedback.

@m-reza-rahman m-reza-rahman marked this pull request as draft June 24, 2024 18:12
@m-reza-rahman m-reza-rahman changed the title Add all positive cases for tomee Add all positive cases for TomEE Jun 26, 2024
@jannahsherif jannahsherif marked this pull request as ready for review June 27, 2024 13:23
@jannahsherif
Copy link
Contributor Author

Hello I just committed my new changes. After reviewing the feedback I add docker support and added the deleted cases

Copy link
Contributor

@m-reza-rahman m-reza-rahman left a comment

Choose a reason for hiding this comment

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

Please order by SE version (starting with the lowest), runtime/none (alphabetic, starting with none), EE version (starting with the lowest), profile (full, web, core), and Docker (no, yes).

So like this:

  • Run Archetype for EE 8 Web Profile, SE 8, Payara
  • Run Archetype for EE 8 Web Profile, SE 8, Payara, with Docker

Please order your part and the file overall - including existing cases. Confusing to read and evaluate otherwise for me.

@m-reza-rahman m-reza-rahman marked this pull request as draft June 27, 2024 23:49
@m-reza-rahman
Copy link
Contributor

Please resolve conflicts with latest commits to master.

@jannahsherif
Copy link
Contributor Author

Hello I tried implement the reviews and organizing the cases correctly

@m-reza-rahman
Copy link
Contributor

At the bare minimum, the issue you need to resolve now are the conflicts before I can review again. Those will happen and are normal in a multi-developer project.

@jannahsherif jannahsherif marked this pull request as ready for review July 2, 2024 15:01
@jannahsherif
Copy link
Contributor Author

hi i reviewed the conflicts

@m-reza-rahman
Copy link
Contributor

OK. I will review if I have some time during the work week. Otherwise, may need to wait until the weekend.

Copy link
Contributor

@m-reza-rahman m-reza-rahman left a comment

Choose a reason for hiding this comment

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

Please double check ordering/duplication. It is still incorrect. Check the latest successful merges for reference if needed.

@m-reza-rahman m-reza-rahman marked this pull request as draft July 2, 2024 19:23
@jannahsherif jannahsherif marked this pull request as ready for review July 4, 2024 12:01
Copy link
Contributor

@m-reza-rahman m-reza-rahman left a comment

Choose a reason for hiding this comment

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

This PR should be focused on TomEE. Please remove changes related to WildFly.

@m-reza-rahman m-reza-rahman marked this pull request as draft July 4, 2024 19:27
@@ -320,6 +346,8 @@ jobs:
- name: Run Archetype for EE 8 Web Profile, SE 11, WildFly, with Docker
run: |
mvn archetype:generate -DinteractiveMode=false -DaskForDefaultPropertyValues=false -DarchetypeGroupId=org.eclipse.starter -DarchetypeArtifactId=jakarta-starter -DarchetypeVersion=2.3.0-SNAPSHOT -DjakartaVersion=8 -Dprofile=web -DjavaVersion=11 -Druntime=wildfly -Ddocker=yes -DoutputDirectory=app/wildfly -Dgoals="clean package"
docker build -t test-image app/wildfly/jakartaee-hello-world
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove this. This is not related to TomEE.

@@ -330,6 +358,8 @@ jobs:
- name: Run Archetype for EE 10, SE 11, WildFly, with Docker
run: |
mvn archetype:generate -DinteractiveMode=false -DaskForDefaultPropertyValues=false -DarchetypeGroupId=org.eclipse.starter -DarchetypeArtifactId=jakarta-starter -DarchetypeVersion=2.3.0-SNAPSHOT -DjakartaVersion=10 -Dprofile=full -DjavaVersion=11 -Druntime=wildfly -Ddocker=yes -DoutputDirectory=app/wildfly -Dgoals="clean package"
docker build -t test-image app/wildfly/jakartaee-hello-world
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove this. This is not related to TomEE.

@@ -340,6 +370,8 @@ jobs:
- name: Run Archetype for EE 10 Web Profile, SE 11, WildFly, with Docker
run: |
mvn archetype:generate -DinteractiveMode=false -DaskForDefaultPropertyValues=false -DarchetypeGroupId=org.eclipse.starter -DarchetypeArtifactId=jakarta-starter -DarchetypeVersion=2.3.0-SNAPSHOT -DjakartaVersion=10 -Dprofile=web -DjavaVersion=11 -Druntime=wildfly -Ddocker=yes -DoutputDirectory=app/wildfly -Dgoals="clean package"
docker build -t test-image app/wildfly/jakartaee-hello-world
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove this. This is not related to TomEE.

@@ -350,10 +382,12 @@ jobs:
- name: Run Archetype for EE 10 Core Profile, SE 11, WildFly, with Docker
run: |
mvn archetype:generate -DinteractiveMode=false -DaskForDefaultPropertyValues=false -DarchetypeGroupId=org.eclipse.starter -DarchetypeArtifactId=jakarta-starter -DarchetypeVersion=2.3.0-SNAPSHOT -DjakartaVersion=10 -Dprofile=core -DjavaVersion=11 -Druntime=wildfly -Ddocker=yes -DoutputDirectory=app/wildfly -Dgoals="clean package"
docker build -t test-image app/wildfly/jakartaee-hello-world
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove this. This is not related to TomEE.

rm -rf app/wildfly

- name: Set up Java SE 17
uses: actions/setup-java@v3
uses: actions/setup-java@v4
Copy link
Contributor

Choose a reason for hiding this comment

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

Please pull the latest from master. This diff should disappear.

@@ -497,12 +531,32 @@ jobs:
run: |
mvn archetype:generate -DinteractiveMode=false -DaskForDefaultPropertyValues=false -DarchetypeGroupId=org.eclipse.starter -DarchetypeArtifactId=jakarta-starter -DarchetypeVersion=2.3.0-SNAPSHOT -DjakartaVersion=10 -Dprofile=core -DjavaVersion=17 -Druntime="open-liberty" -Ddocker=yes -DoutputDirectory="app/open-liberty" -Dgoals="clean package"
rm -rf app/open-liberty

Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this white space change needed?

with:
distribution: "temurin"
java-version: 11

- name: Cache Maven packages
uses: actions/cache@v3
uses: actions/cache@v4
Copy link
Contributor

Choose a reason for hiding this comment

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

Please pull the latest from master. This diff should disappear.

- name: Setup Java
uses: actions/setup-java@v3
uses: actions/setup-java@v4
Copy link
Contributor

Choose a reason for hiding this comment

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

Please pull the latest from master. This diff should disappear.

build-ui:
runs-on: ubuntu-20.04
steps:
- uses: actions/checkout@v3
- uses: actions/checkout@v4
Copy link
Contributor

Choose a reason for hiding this comment

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

Please pull the latest from master. This diff should disappear.

@@ -516,6 +570,8 @@ jobs:
- name: Run Archetype for EE 8, SE 17, WildFly, with Docker
run: |
mvn archetype:generate -DinteractiveMode=false -DaskForDefaultPropertyValues=false -DarchetypeGroupId=org.eclipse.starter -DarchetypeArtifactId=jakarta-starter -DarchetypeVersion=2.3.0-SNAPSHOT -DjakartaVersion=8 -Dprofile=full -DjavaVersion=17 -Druntime=wildfly -Ddocker=yes -DoutputDirectory=app/wildfly -Dgoals="clean package"
docker build -t test-image app/wildfly/jakartaee-hello-world
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove this. This is not related to TomEE.

@@ -526,6 +582,8 @@ jobs:
- name: Run Archetype for EE 8 Web Profile, SE 17, WildFly, with Docker
run: |
mvn archetype:generate -DinteractiveMode=false -DaskForDefaultPropertyValues=false -DarchetypeGroupId=org.eclipse.starter -DarchetypeArtifactId=jakarta-starter -DarchetypeVersion=2.3.0-SNAPSHOT -DjakartaVersion=8 -Dprofile=web -DjavaVersion=17 -Druntime=wildfly -Ddocker=yes -DoutputDirectory=app/wildfly -Dgoals="clean package"
docker build -t test-image app/wildfly/jakartaee-hello-world
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove this. This is not related to TomEE.

@@ -536,6 +594,8 @@ jobs:
- name: Run Archetype for EE 10, SE 17, WildFly, with Docker
run: |
mvn archetype:generate -DinteractiveMode=false -DaskForDefaultPropertyValues=false -DarchetypeGroupId=org.eclipse.starter -DarchetypeArtifactId=jakarta-starter -DarchetypeVersion=2.3.0-SNAPSHOT -DjakartaVersion=10 -Dprofile=full -DjavaVersion=17 -Druntime=wildfly -Ddocker=yes -DoutputDirectory=app/wildfly -Dgoals="clean package"
docker build -t test-image app/wildfly/jakartaee-hello-world
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove this. This is not related to TomEE.

@@ -546,6 +606,8 @@ jobs:
- name: Run Archetype for EE 10 Web Profile, SE 17, WildFly, with Docker
run: |
mvn archetype:generate -DinteractiveMode=false -DaskForDefaultPropertyValues=false -DarchetypeGroupId=org.eclipse.starter -DarchetypeArtifactId=jakarta-starter -DarchetypeVersion=2.3.0-SNAPSHOT -DjakartaVersion=10 -Dprofile=web -DjavaVersion=17 -Druntime=wildfly -Ddocker=yes -DoutputDirectory=app/wildfly -Dgoals="clean package"
docker build -t test-image app/wildfly/jakartaee-hello-world
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove this. This is not related to TomEE.

@@ -556,21 +618,22 @@ jobs:
- name: Run Archetype for EE 10 Core Profile, SE 17, WildFly, with Docker
run: |
mvn archetype:generate -DinteractiveMode=false -DaskForDefaultPropertyValues=false -DarchetypeGroupId=org.eclipse.starter -DarchetypeArtifactId=jakarta-starter -DarchetypeVersion=2.3.0-SNAPSHOT -DjakartaVersion=10 -Dprofile=core -DjavaVersion=17 -Druntime=wildfly -Ddocker=yes -DoutputDirectory=app/wildfly -Dgoals="clean package"
docker build -t test-image app/wildfly/jakartaee-hello-world
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove this. This is not related to TomEE.

Copy link
Contributor

@m-reza-rahman m-reza-rahman left a comment

Choose a reason for hiding this comment

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

I have now added specific in-place feedback for you. Please address them. Then I can review hopefully one last time before merge.

@m-reza-rahman m-reza-rahman marked this pull request as draft July 7, 2024 21:45
@jannahsherif
Copy link
Contributor Author

Thank you for the detailed explanation! I resolved all the conflicts but I am running into a problem with the space changes. If i remove the extra space it highlights it in red. If I readd it, it shows one line red then the one after green. How can i get it not be red or green?

@m-reza-rahman
Copy link
Contributor

It looks honestly like you have some sync issues. What I suggest is closing this PR, saving aside your changes, forking fresh again, re-apply your changes, and then open a new PR. That might be easiest at this point.

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.

None yet

3 participants