search

euranova / code_retreat

Materials for ENX code retreat. If you don't find your language here, check https://github.com/swkBerlin/kata-bootstraps
MIT License
7 stars 12 forks source link

Clarify and lighten ruby scaffold #51

Closed ThomasCollignon closed 2 years ago

ThomasCollignon commented 2 years ago

I've grouped the changes by relevant commits. Feel free to challenge every change.

ThomasCollignon commented 2 years ago

I've discovered this repo thanks to @Malian, one of the contributors, who uses it for code retreat meetings in our company, Belighted. Personally I also use it to learn new languages basic scaffolds, and minimal Docker integration.

About the scripts/test and scripts/build, I've removed them because I didn't read the main README, shame on me.

I see that you've pushed some changes, I'll check them asap and adapt or remove this PR consequently.

jbruggem commented 2 years ago

I've removed them because I didn't read the main README, shame on me.

You're not to blame here, I added those explanations yesterday following your PR. So on the contrary, many thanks for the nudge :).

I'll check them asap and adapt or remove this PR consequently.

Thanks ! I haven't changed much in the ruby folder, so there still are two competing scripts to run ruby in docker, and that indeed needs to be simplified. So the clarification and lightening is still welcome and necessary :).

Malian commented 2 years ago

:wave: @jbruggem

ThomasCollignon commented 2 years ago

@jbruggem I have two more questions:

jbruggem commented 2 years ago

Why ENABLE_TTY=${ENABLE_TTY:--t} instead of just -t ? And actually I can't figure out why/how/who would set ENABLE_TTY.

See scaffolds/test_all.sh. The objective is to be able to disable TTY when we're testing all the scripts. The name of the env var is probably not ideal given what it's used for :).

What use do you make of the binstubs generated in the docker-home folder ?

I'm not an expert in Ruby, so I have to guess a bit here. I suppose that when I wrote the ruby-env script, I wanted:

I suppose that this is why HOME, GEM_HOME and BUNDLE_PATH are set. But let's be clear: I don't know much about ruby at all :). As long as we have something that's easy to maintain, consistent with the rest and easy to use for newbies, it's all good :).

ThomasCollignon commented 2 years ago

Sorry for the delay.

I've seen no perf improvement using binstubs, so I've removed it. The goal is to keep only one docker run script: it's easier to understand.

jbruggem commented 2 years ago

Thanks for the contribution @ThomasCollignon !