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

CHE-3774 Add Eclipse Vert.x stack to default assembly #3843

Merged
merged 2 commits into from
Feb 6, 2017

Conversation

sbryzak
Copy link
Contributor

@sbryzak sbryzak commented Jan 21, 2017

Signed-off-by: Shane Bryzak sbryzak@redhat.com

This pull request adds the Eclipse Vert.x stack to the default Che assembly.

What issues does this PR fix or reference?

CHE-3774

Changelog

Adds the Eclipse Vert.x stack to the default Che assembly.

Release note

We've added a new stack to for the Eclipse Vert.x project. This is a very popular toolkit for building reactive applications on the JVM. You can now build vert.x applications by simply creating a workspace from the vert.x stack - no configuration necessary!

@codenvy-ci
Copy link

Can one of the admins verify this patch?

Copy link
Contributor

@l0rd l0rd left a comment

Choose a reason for hiding this comment

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

Thank you @sbryzak for this PR! That's great to have a vertX stack. I'm wondering if we could also add the Dockerfile to https://github.com/eclipse/che-dockerfiles and double check with @dharmit if registry.centos.org/dharmit/vertx is the definitive repository or if we want to move it to something like registry.centos.org/che-stacks/vertx.

@slemeur
Copy link
Contributor

slemeur commented Jan 23, 2017

Thanks @sbryzak, that's really nice to have a vert.x stack.

I would like to suggest two things to associate to this PR:
1- We could provide to end user a sample vert.x project which help them to get started.
2- We could maybe provide a nice tutorial about Vert.x in Che - this could maybe be referenced on Vert.x documentation too.

wdyt @sbryzak ?
My guess is that 1 is important and 2 a nice to have ;)

@slemeur slemeur added the kind/enhancement A feature request - must adhere to the feature request template. label Jan 23, 2017
@slemeur
Copy link
Contributor

slemeur commented Jan 23, 2017

PR added to release note.
Added doc issue: https://github.com/eclipse/che-docs/issues/89

@sbryzak
Copy link
Contributor Author

sbryzak commented Jan 24, 2017

@l0rd I've e-mailed Dharmit to confirm which repo should be the definitive one. @slemeur I've also asked whether a sample project and tutorial can be provided, will update back here once I get a response.

@sbryzak
Copy link
Contributor Author

sbryzak commented Jan 24, 2017

@slemeur The following example may be used, what is the best/proper way to include this in Che?

https://github.com/cescoffier/vertx-with-che

@sbryzak
Copy link
Contributor Author

sbryzak commented Jan 30, 2017

This pull request has been updated with the definitive repository registry.centos.org/che-stacks/vertx. It should be fine to merge it now.

@benoitf
Copy link
Contributor

benoitf commented Jan 30, 2017

ci-build

@codenvy-ci
Copy link

}],
"source": {
"type": "image",
"origin": "registry.centos.org/che-stacks/vertx"
Copy link
Contributor

Choose a reason for hiding this comment

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

Choose a reason for hiding this comment

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

I don't know what's in the provided image. However, the later versions the better for Vert.x. These JDK and Maven versions are the lower bounds, any more up to date version would be fine.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know what's in the provided image.

@cescoffier: you can find the Dockerfile and other files for the image here.

Choose a reason for hiding this comment

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

Thanks, @dharmit. So it's Maven 3.3 (the one we used).

Note for me: don't forget to update the docker file after Vert.x releases.

}
},
"recipe": {
"location": "registry.centos.org/dharmit/vertx",
Copy link
Contributor

Choose a reason for hiding this comment

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

@sbryzak should we replace this with registry.centos.org/che-stacks/vertx as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, I missed that one - updated now.

@benoitf benoitf added this to the 5.3.0 milestone Feb 6, 2017
@benoitf benoitf removed the request for review from JamesDrummond February 6, 2017 11:09
@benoitf benoitf added status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. and removed status/pending-merge labels Feb 6, 2017
@benoitf benoitf removed this from the 5.3.0 milestone Feb 6, 2017
Signed-off-by: Shane Bryzak <sbryzak@redhat.com>
@l0rd
Copy link
Contributor

l0rd commented Feb 6, 2017

@TylerJewell we added vert.x image in the CLI image-stacks lists for version 5.3.0 and nightly. Is it good for you?

@@ -20,3 +20,4 @@ eclipse/ubuntu_jdk8
eclipse/ubuntu_jre
eclipse/ubuntu_python
eclipse/ubuntu_wildfly8
registry.centos.org/che-stacks/vertx
Copy link
Contributor

Choose a reason for hiding this comment

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

missing the newline

@benoitf
Copy link
Contributor

benoitf commented Feb 6, 2017

@l0rd adding newline at end of file in dockerfiles/cli/version/5.3.0/images-stacks and I think you can merge

Signed-off-by: Mario Loriedo <mloriedo@redhat.com>
@l0rd
Copy link
Contributor

l0rd commented Feb 6, 2017

@benoitf Thanks!

@l0rd l0rd merged commit 11da7a2 into eclipse-che:master Feb 6, 2017
@bmicklea bmicklea added this to the 5.3.0 milestone Feb 8, 2017
@bmicklea
Copy link

bmicklea commented Feb 8, 2017

Please remember to add the correct milestone when merging PRs - it makes it easier for PM to track for changelog and release notes.

@l0rd
Copy link
Contributor

l0rd commented Feb 9, 2017

@bmicklea ok will do that

@JamesDrummond JamesDrummond removed the status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. label Feb 17, 2017
@JamesDrummond JamesDrummond mentioned this pull request Feb 17, 2017
9 tasks
JPinkney pushed a commit to JPinkney/che that referenced this pull request Aug 17, 2017
Signed-off-by: Shane Bryzak <sbryzak@redhat.com>
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement A feature request - must adhere to the feature request template.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants