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

adding example java docs #1153

Merged
merged 3 commits into from
Jan 14, 2019
Merged

Conversation

sherl0cks
Copy link
Contributor

What is the purpose of this change? What does it change?

Was the change discussed in an issue?

fixes #???
#839, but does not completely resolve. #839 suggests that the getting started page include java. This makes sense, but is a more involved rewrite. I would suggest the team 1) split 839 and 2) use the example from the katacoda to rewrite the getting started docs for java and node

How to test changes?

It's a doc change.

A couple notes:

  • due to Support Context Dir Like oc new-app #1152, I cannot use any of the red hat repos for java examples as they need --context-dir support. Thus, I'm using Spring Pet Clinic here.
  • I think it would be good to refactor this whole page to include all commands needed for a working example (e.g. odo push & odo url create). Creating the component is only one part of the job and makes the docs a bit confusing.

@codeclimate
Copy link

codeclimate bot commented Dec 19, 2018

Code Climate has analyzed commit c5e3bc8 and detected 0 issues on this pull request.

View more on Code Climate.

@AppVeyorBot
Copy link

@codecov
Copy link

codecov bot commented Dec 19, 2018

Codecov Report

Merging #1153 into master will increase coverage by 0.68%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #1153      +/-   ##
=========================================
+ Coverage   42.51%   43.2%   +0.68%     
=========================================
  Files          28      29       +1     
  Lines        4081    4111      +30     
=========================================
+ Hits         1735    1776      +41     
+ Misses       2166    2147      -19     
- Partials      180     188       +8
Impacted Files Coverage Δ
pkg/occlient/utils.go 100% <0%> (ø)
pkg/occlient/occlient.go 47.69% <0%> (+0.59%) ⬆️
pkg/util/util.go 60.65% <0%> (+1.83%) ⬆️
pkg/component/watch.go 70.58% <0%> (+2.61%) ⬆️
pkg/service/service.go 60.71% <0%> (+2.64%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7ee2371...c5e3bc8. Read the comment docs.

@geoand
Copy link
Contributor

geoand commented Dec 19, 2018

@cdrage Weren't you working on something similar?

docs/examples.md Outdated
@@ -1,6 +1,6 @@
# Examples

Odo is compatible with any language or runtime listed within OpenShift's catalog of component types.
Odo is compatible with any language or runtime listed within OpenShift's catalog of component types. To create url for component, see the [cli reference for `odo url`](cli-reference.md#url).
Copy link
Member

Choose a reason for hiding this comment

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

Rather than showing how to create url, the wording could possibly be better with this:

In order to access a component, you may create a URL using odo url create

Or something along those lines

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How about now?

@cdrage
Copy link
Member

cdrage commented Dec 19, 2018

@geoand

Sort of. I'm creating a doc on how to setup a database and link it to a component. How to push both your back-end and front-end systems.

@geoand
Copy link
Contributor

geoand commented Dec 19, 2018

Thanks @cdrage

@sherl0cks
Copy link
Contributor Author

@cdrage where can I follow along the component / database docs? I need them personally and would also like to contribute back.

@sherl0cks
Copy link
Contributor Author

This thread also makes me thing we should look at #1155

@geoand
Copy link
Contributor

geoand commented Dec 19, 2018

@sherl0cks although it's definitely not documentation, we have various integration tests that use java.

I think the one that @cdrage is planning his docs update on is this and the code that uses it can be found here

@AppVeyorBot
Copy link

git clone https://github.com/spring-projects/spring-petclinic.git
cd spring-petclinic
mvn package
odo create java test3 --binary target/*.jar
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure you want to push all jars all the time. Usually, you only push the uber-jar…

Copy link
Contributor Author

@sherl0cks sherl0cks Dec 20, 2018

Choose a reason for hiding this comment

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

Generally speaking, when packaging as an uber jar, you will only get a single jar in target - the uber jar. If you hard code the name of that, simple things like maven version changes can break automation

Copy link
Contributor

Choose a reason for hiding this comment

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

Generally true, yes but still risky :)

Copy link
Contributor Author

@sherl0cks sherl0cks Dec 20, 2018

Choose a reason for hiding this comment

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

OK - how about odo create java petclinic --binary target/spring-petclinic-*.jar ? I'm trying to avoid spring reving the version and breaking the example.

@sherl0cks
Copy link
Contributor Author

bump

@geoand
Copy link
Contributor

geoand commented Jan 7, 2019

@cdrage Can you please take a look at this when you have some time? Thanks

@surajnarwade
Copy link
Contributor

surajnarwade commented Jan 14, 2019

cc @cdrage @metacosm , your 💚 is needed here 😄

Copy link
Member

@cdrage cdrage left a comment

Choose a reason for hiding this comment

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

Docs LGTM. Just a minor change.

docs/examples.md Outdated
@@ -80,6 +89,18 @@ Build and run WildFly applications on CentOS 7. For more information about using

## Binary example

### java

Java can be used to deploy binary artifact
Copy link
Member

Choose a reason for hiding this comment

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

Java may be used to deploy a binary artifact, for example:

Or something similar ^^

docs/examples.md Outdated
@@ -88,3 +109,4 @@ WildFly can deploy a binary application.
wget -O example.war 'https://github.com/appuio/hello-world-war/blob/master/repo/ch/appuio/hello-world-war/1.0.0/hello-world-war-1.0.0.war?raw=true'
odo create wildfly --binary example.war
```

Copy link
Member

Choose a reason for hiding this comment

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

newline added by mistake?

@sherl0cks
Copy link
Contributor Author

sherl0cks commented Jan 14, 2019

@cdrage done.

May I suggest the team look at a more optimistic merging approach? A one month cycle time for a small docs update with changes for newlines isn't the greatest new contributor experience. I realize the holidays got in there, but still something to consider.

@AppVeyorBot
Copy link

@cdrage
Copy link
Member

cdrage commented Jan 14, 2019

LGTM!

@cdrage
Copy link
Member

cdrage commented Jan 14, 2019

@sherl0cks Yeah, we need to improve on merging. We talked about this at the face-to-face meeting we had last week and we're going to be creating a newly updated DOC for merging / PR review procedures.

I send the link to the team too! http://hintjens.com/blog:106

@cdrage cdrage merged commit 3b0227a into redhat-developer:master Jan 14, 2019
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.

6 participants