moodlehq / moodle-docker

A docker environment for moodle developers
GNU General Public License v3.0
373 stars 244 forks source link

Split browser name before in case it uses tags #265

Closed crazyserver closed 1 year ago

stronk7 commented 1 year ago

Hi @crazyserver ,

change looks ok, good stuff!

Only question I've here is if, maybe, we also have to move the "split" code in bin/moodle-docker-compose.cmd. While the code managing the app for Windows is different from the Unix one... surely there we should be also looking for the MOODLE_DOCKER_BROWSER_NAME, and not for the whole MOODLE_DOCKER_BROWSER that can come with tags.

I think that, with that little detail covered... this will be perfect to be landed.

Ciao :-)

crazyserver commented 1 year ago

Thanks Eloy! Can you review again? :-)

stronk7 commented 1 year ago

I've relaunched 2-3 jobs that failed (hopefully random ones). Code-wise, it seems that now everything is already in place, with the split happening before the browser name is used, so all ok.

stronk7 commented 1 year ago

Sold, thanks!