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

make deploy script mac os friendly #6560

Merged
merged 7 commits into from
Oct 4, 2017
Merged

Conversation

riuvshin
Copy link
Contributor

@riuvshin riuvshin commented Oct 3, 2017

What does this PR do?

synchronize deploy script from rh-che, make it mac os friendly

@riuvshin riuvshin self-assigned this Oct 3, 2017
@riuvshin riuvshin changed the base branch from master to che-multiuser October 3, 2017 16:22
@riuvshin riuvshin added kind/task Internal things, technical debt, and to-do tasks to be performed. sprint/current team/production labels Oct 3, 2017
@benoitf benoitf added target/branch Indicates that a PR will be merged into a branch other than master. status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. labels Oct 3, 2017
@codenvy-ci
Copy link

@l0rd
Copy link
Contributor

l0rd commented Oct 4, 2017

@riuvshin why Mac Os friendly? And you are updating it here and not in master branch because you are supposed to merge it to master by the end of the week anyway right?

sed "s| keycloak-oso-endpoint:.*| keycloak-oso-endpoint: ${KEYCLOAK_OSO_ENDPOINT}|" | \
sed "s| keycloak-github-endpoint:.*| keycloak-github-endpoint: ${KEYCLOAK_GITHUB_ENDPOINT}|" | \
sed "s/ keycloak-disabled:.*/ keycloak-disabled: \"${CHE_KEYCLOAK_DISABLED}\"/" | \
if [ "${CHE_LOG_LEVEL}" == "DEBUG" ]; then sed "s/ log-level: \"INFO\"/ log-level: \"DEBUG\"/" ; else cat -; fi | \
sed "$MULTI_USER_REPLACEMENT_STRING" | \
if [ "${ENABLE_SSL}" == "false" ]; then sed "s/ che-openshift-secure-routes: \"true\"/ che-openshift-secure-routes: \"false\"/" ; else cat -; fi | \
if [ "${ENABLE_SSL}" == "false" ]; then sed "s/ che-secure-external-urls: \"true\"/ che-secure-external-urls: \"false\"/" ; else cat -; fi | \

Choose a reason for hiding this comment

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

You can combine these lines

# ----------------
# helper functions
# ----------------
append_after_match() {

Choose a reason for hiding this comment

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

Can you describe what it is used for in line comments

Copy link
Contributor

@davidfestal davidfestal left a comment

Choose a reason for hiding this comment

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

OK for me, unless it might have been 2 PRs: one PR to make it macOS friendy (only related to multiuser additions => on the che-multiuser branch), and one PR to synchronize deploy scripts, that could have been made on master.

@riuvshin
Copy link
Contributor Author

riuvshin commented Oct 4, 2017

@l0rd there was used variation of sed that does not work on MAC OS. My goal to have script that can deploy multiuser branch to ocp. for master I use deploy script from rh-che project

@riuvshin
Copy link
Contributor Author

riuvshin commented Oct 4, 2017

agree with @davidfestal I will introduce separate PRs

@riuvshin
Copy link
Contributor Author

riuvshin commented Oct 4, 2017

@davidfestal @l0rd here is sync PR to master #6566 once it will be merged I will update this PR

@riuvshin riuvshin changed the title synchronize deploy script from rh-che, make it mac os friendly make deploy script mac os friendly Oct 4, 2017
@riuvshin
Copy link
Contributor Author

riuvshin commented Oct 4, 2017

@l0rd @davidfestal PR is updated :)

@codenvy-ci
Copy link

@riuvshin riuvshin merged commit f055801 into che-multiuser Oct 4, 2017
@benoitf benoitf removed the status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. label Oct 4, 2017
@riuvshin riuvshin deleted the multiuser_fix_script branch October 5, 2017 08:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/task Internal things, technical debt, and to-do tasks to be performed. sprint/current target/branch Indicates that a PR will be merged into a branch other than master.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants