shell script to start sencha

[riedde]

Description, Context and related Issue

This PR contains a very small shell script to run a docker containter with sencha for building the edirom

How Has This Been Tested?

I removed the absolute paths and added public valiables. It works on my machine but should work on every machine (Mac).

Types of changes

• New feature (non-breaking change which adds functionality)

Checklist

• My change requires a change to the documentation.
• I have performed a self-review of my code

[riedde]

@krHERO To be discussed with the developer team: We can also integrate this script into the build process.

[daniel-jettka]

@krHERO To be discussed with the developer team: We can also integrate this script into the build process.

I like the idea to integrate this into the build process.
build.sh could test if sencha and ant are available locally and if not then start the build process using the container.

Small addition starts the build process in the container directly:
docker run --rm -it -v `pwd`:/app --name sencha ghcr.io/bwbohl/sencha-cmd:latest /bin/bash -c "./build.sh testing; exit"

What do you think?

[riedde]

great idea. I thought something similar. Maybe we can use testing as default if no parameter is set? I have done something similar with the password (for local development use) here: https://github.com/Baumann-Digital/baudi-docker/blob/main/build-run-docker.sh

[peterstadler]

My suggestion would be to not provide this as a shell script but rather add this to the ant build.xml.
Since we already have ant in place I think it'd be beneficial to employ this tool for tying together all the build, test, and run steps.

This also includes current build.sh which we (imho) should get rid off.

[riedde]

My intention was to share my way of working, an this was just some addition to build.sh, but I also think we should try to extent the ant build.xml see #440

[riedde]

I converted this to draft until the ant way is explored. I'm intrested in working on that to make a proposal.

[roewenstrunk]

My suggestion would be to not provide this as a shell script but rather add this to the ant build.xml. Since we already have ant in place I think it'd be beneficial to employ this tool for tying together all the build, test, and run steps.

This also includes current build.sh which we (imho) should get rid off.

It is an interesting question which tools we expect developers to use. Benni's Docker image has Ant integrated. It would therefore be possible to develop on the Edirom without having Ant installed locally.

[riedde]

Are we talking about the same? I am talking about Benni‘s Sencha-Image not about his Edirom-Image. So what about a short discussion on this at the next developer’s meeting? Maybe we already have some good Workflows and just have to share or combine them?

[roewenstrunk]

Are we talking about the same? I am talking about Benni‘s Sencha-Image not about his Edirom-Image. So what about a short discussion on this at the next developer’s meeting? Maybe we already have some good Workflows and just have to share or combine them?

I was talking about the Sencha-CMD image, too. There is Ant integrated. So technically you don't need Ant locally installed to build Edirom Online.

[riedde]

Are we talking about the same? I am talking about Benni‘s Sencha-Image not about his Edirom-Image. So what about a short discussion on this at the next developer’s meeting? Maybe we already have some good Workflows and just have to share or combine them?

I was talking about the Sencha-CMD image, too. There is Ant integrated. So technically you don't need Ant locally installed to build Edirom Online.

Now, I got it. I guess this is why my first proposal was to integrate the docker commands into build.sh

[roewenstrunk]

@krHERO To be discussed with the developer team: We can also integrate this script into the build process.

I like the idea to integrate this into the build process. build.sh could test if sencha and ant are available locally and if not then start the build process using the container.

Small addition starts the build process in the container directly: docker run --rm -it -v pwd:/app --name sencha ghcr.io/bwbohl/sencha-cmd:latest /bin/bash -c "./build.sh testing; exit"

What do you think?

@riedde, if you follow Daniel's suggestion, the docker command can't be integrated into build.sh because build.sh is called inside the docker container. So you would need another shell script for calling docker.

[riedde]

Thanks for the hint! I'm going to investigate some solutions and present them at the next meeting.

[daniel-jettka]

Proposing small change to current script

• flag "-d" starts docker container and in there the script is executed in the former way

[riedde]

Tested on my machine. Works fine! Thank you @daniel-jettka for your help!

[peterstadler]

@riedde please do not hit the "update branch" button and merge dev into this branch. This will create "ugly" history for it inserts all those unnecessary merge commits. It's better to rebase the branch but beware that you'll need to eventually reset hard your local clone.

[riedde]

@peterstadler Okay, I won't use this button again, but rebasing is not my favourit way of working. It cost a lot of time because there are always conflicts that are mostly hard to handle. Especially when there is to much code formatting.

[peterstadler]

I think the exit is not really needed?

Suggested change

exit

[peterstadler]

instead of hard coding the "testing" flag I suggest to read this from the initial command line arguments and pass it on to the Docker command

Suggested change

	docker run --rm -it -v `pwd`:/app --name sencha ghcr.io/bwbohl/sencha-cmd:latest /bin/bash -c "./build.sh testing; exit"
	shift $((OPTIND-1)) # remove -d from the list of input arguments
	OPTIONS=${@} # set a variable OPTIONS with the remaining input arguments to pass to the build command.
	# run docker
	docker run --rm -it -v `pwd`:/app --name sencha ghcr.io/bwbohl/sencha-cmd:latest ./build.sh $OPTIONS"

With that you can also do e.g. ./build.sh -d production

Additionally, I removed the exit which is not needed imho

[peterstadler]

This is not really needed, imho

Suggested change

exit