Skip to content

Makefile: remove ruby from building steps#61

Merged
eguzki merged 1 commit intomasterfrom
makefile-not-using-ruby
Nov 14, 2018
Merged

Makefile: remove ruby from building steps#61
eguzki merged 1 commit intomasterfrom
makefile-not-using-ruby

Conversation

@eguzki
Copy link
Copy Markdown
Member

@eguzki eguzki commented Nov 9, 2018

Remove requirement from the building environment to have ruby installed. Docker will be the only tool required to build the project.

@eguzki eguzki requested a review from unleashed November 9, 2018 16:17
Comment thread Makefile Outdated

.PHONY: ci-build
ci-build: export APISONATOR_REL?=v$(shell ruby -r$(PROJECT_PATH)/lib/3scale/backend/version -e "puts ThreeScale::Backend::VERSION")
ci-build: export APISONATOR_REL?=v$(shell docker run --rm -e RBENV_VERSION='2.3.6' -v $(PROJECT_PATH):/tmp/apisonator:z \
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can we remove the RBENV_VERSION there? If not, perhaps we can just specify 2.3 or even 2 - otherwise we will be depending on the CI image always having a 2.3.6 release.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Is there any way to disable rbenv or use default version? Otherwise, using another image could work, like centos/ruby-23-centos7

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It should be ready to read the .ruby-version in the app root and use that to get the version (ie. do not define the env variable and first cd into the directory before running ruby).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

also, using a different image would work but would not be practical as another image would need to be download just for this and you would still encode the version here instead of using the app/ci image version

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

used .ruby-version of the project 👍

@eguzki eguzki force-pushed the makefile-not-using-ruby branch from 192f04a to 39ba207 Compare November 10, 2018 15:09
Copy link
Copy Markdown
Contributor

@unleashed unleashed left a comment

Choose a reason for hiding this comment

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

LGTM - it appears CircleCI broke?

@eguzki eguzki closed this Nov 12, 2018
@eguzki eguzki reopened this Nov 12, 2018
@eguzki eguzki closed this Nov 12, 2018
@eguzki eguzki reopened this Nov 12, 2018
@eguzki eguzki force-pushed the makefile-not-using-ruby branch from 39ba207 to 09fc9b6 Compare November 12, 2018 14:54
@eguzki
Copy link
Copy Markdown
Member Author

eguzki commented Nov 14, 2018

bors r=@unleashed

bors Bot added a commit that referenced this pull request Nov 14, 2018
61: Makefile: remove ruby from building steps r=unleashed a=eguzki

Remove requirement from the building environment to have ruby installed. Docker will be the only tool required to build the project. 

Co-authored-by: Eguzki Astiz Lezaun <eguzki.astiz@gmail.com>
@eguzki eguzki merged commit 0c4ec6c into master Nov 14, 2018
@bors bors Bot deleted the makefile-not-using-ruby branch November 14, 2018 11:40
@bors
Copy link
Copy Markdown
Contributor

bors Bot commented Nov 14, 2018

Timed out

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.

2 participants