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

Added runtime openshift template and documentation. #102

Merged
merged 6 commits into from Aug 22, 2017

Conversation

Projects
None yet
3 participants
@aslicerh
Member

aslicerh commented Aug 9, 2017

No description provided.

@aslicerh

This comment has been minimized.

Show comment
Hide comment
@aslicerh

aslicerh Aug 9, 2017

Member

Note: This won't actually work until the 2.0 containers are available. But you can grab a local copy and change the container it points to test it until then.

Member

aslicerh commented Aug 9, 2017

Note: This won't actually work until the 2.0 containers are available. But you can grab a local copy and change the container it points to test it until then.

@aslicerh aslicerh requested review from tmds and jerboaa Aug 9, 2017

}
}
],
"parameters": [

This comment has been minimized.

@tmds

tmds Aug 10, 2017

Member

I'd be nice if this was the same parameters as the dotnet-examples. No parameters added/removed. Just the way the image is built should be different.

@tmds

tmds Aug 10, 2017

Member

I'd be nice if this was the same parameters as the dotnet-examples. No parameters added/removed. Just the way the image is built should be different.

@jerboaa

Looks mostly good. Early feedback below. I'll test this throughout today.

Show outdated Hide outdated templates/dotnet-runtime-example.json
Show outdated Hide outdated templates/dotnet-example.json
Show outdated Hide outdated templates/dotnet-runtime-example.json
Show outdated Hide outdated templates/dotnet-runtime-example.json
Show outdated Hide outdated README.md
@jerboaa

This comment has been minimized.

Show comment
Hide comment
@jerboaa

jerboaa Aug 10, 2017

Contributor

Andrew, please make this a PR for master (or create a new one). We are going to merge 2.0-dev into master any time now.

Contributor

jerboaa commented Aug 10, 2017

Andrew, please make this a PR for master (or create a new one). We are going to merge 2.0-dev into master any time now.

@aslicerh

This comment has been minimized.

Show comment
Hide comment
@aslicerh

aslicerh Aug 10, 2017

Member

@jerboaa If you go ahead and do the merge I'll switch the PR over. I don't want to switch it before then.

Member

aslicerh commented Aug 10, 2017

@jerboaa If you go ahead and do the merge I'll switch the PR over. I don't want to switch it before then.

@jerboaa

This comment has been minimized.

Show comment
Hide comment
@jerboaa

jerboaa Aug 10, 2017

Contributor

@aslicerh sounds good.

Contributor

jerboaa commented Aug 10, 2017

@aslicerh sounds good.

aslicerh added some commits Aug 10, 2017

Code review comments:
- Removed image stream tags defined in the runtime template.
- Added user permissions changes to the inline dockerfile in the runtime template.
- Some naming and phrasing changes.
@aslicerh

This comment has been minimized.

Show comment
Hide comment
@aslicerh

aslicerh Aug 10, 2017

Member

Ok, I think this will have addressed everything so far.

Because of the changes to s2i, this one won't even work with the dotnet-beta 2.0 containers without some more edits to a local copy.

Member

aslicerh commented Aug 10, 2017

Ok, I think this will have addressed everything so far.

Because of the changes to s2i, this one won't even work with the dotnet-beta 2.0 containers without some more edits to a local copy.

@jerboaa

Almost good to go. A few minor nits only. Thanks for the update!

Show outdated Hide outdated templates/dotnet-runtime-example.json
Show outdated Hide outdated templates/dotnet-runtime-example.json
Show outdated Hide outdated README.md
Show outdated Hide outdated templates/dotnet-example.json
@jerboaa

Looks good to me.

@tmds

tmds approved these changes Aug 11, 2017

@aslicerh

This comment has been minimized.

Show comment
Hide comment
@aslicerh

aslicerh Aug 11, 2017

Member

Do we want to wait and do this against master after the merge?

Member

aslicerh commented Aug 11, 2017

Do we want to wait and do this against master after the merge?

@aslicerh aslicerh changed the base branch from 2.0-dev to master Aug 11, 2017

@aslicerh

This comment has been minimized.

Show comment
Hide comment
@aslicerh

aslicerh Aug 11, 2017

Member

I guess that determines that question. This PR has been changed to be against master.

Member

aslicerh commented Aug 11, 2017

I guess that determines that question. This PR has been changed to be against master.

@jerboaa jerboaa changed the title from Added runtime openshift template and documentation. to [Do not merge] Added runtime openshift template and documentation. Aug 11, 2017

@jerboaa

This comment has been minimized.

Show comment
Hide comment
@jerboaa

jerboaa Aug 11, 2017

Contributor

We'll merge this once images are available.

Contributor

jerboaa commented Aug 11, 2017

We'll merge this once images are available.

@aslicerh

This comment has been minimized.

Show comment
Hide comment
@aslicerh

aslicerh Aug 11, 2017

Member

I like that plan.

Member

aslicerh commented Aug 11, 2017

I like that plan.

@jerboaa jerboaa changed the title from [Do not merge] Added runtime openshift template and documentation. to Added runtime openshift template and documentation. Aug 22, 2017

@jerboaa jerboaa merged commit 7159122 into redhat-developer:master Aug 22, 2017

1 check failed

RH build bot Build failed.
Details

@aslicerh aslicerh referenced this pull request Aug 22, 2017

Closed

Split build documentation #93

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment