Skip to content

Commit

Permalink
fix(CI) : update docker deps to avoid errors
Browse files Browse the repository at this point in the history
  • Loading branch information
@mrflos
mrflos committed Sep 8, 2023
1 parent 87974b3 commit ee9316f
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 14 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/docker.yml
View file
Expand Up @@ -9,7 +9,7 @@ on:

jobs:
build:
runs-on: ubuntu-latest
runs-on: ubuntu-22.04
steps:
- uses: actions/checkout@v2

Expand Down
14 changes: 5 additions & 9 deletions .github/workflows/phpunit.yml
View file
Expand Up @@ -4,11 +4,11 @@ on:
workflow_dispatch:
inputs:
logLevel:
description: 'Log level'
description: 'Log level'
required: true
default: 'warning'
tags:
description: 'Tags'
description: 'Tags'
push:
branches: [ doryphore, ectoplasme ]
paths:
Expand All @@ -27,15 +27,13 @@ env:

jobs:
build:
# it provides
# see https://github.com/actions/virtual-environments/blob/main/images/linux/Ubuntu1804-README.md
runs-on: ubuntu-18.04
runs-on: ubuntu-22.04

steps:
- uses: actions/checkout@v2
- uses: shivammathur/setup-php@v2
with:
php-version: '7.3'
php-version: '8.2'

This comment has been minimized.

Sign in to view

Copy link
@J9rem

J9rem Sep 8, 2023

Contributor

Salut @mrflos ,

en fait, il y avait une raison volontaire de conserver php 7.3 à cet endroit, c'est pour être sûr que les tests automatiques fonctionnent dans la version php 7.3 car nous garantissons la compatibilité avec cette version.

En mettant à cette endroit php 8.2, nous sommes uniquement sûrs que les tests automatiques sont faits pour php 8.2. Soit il nous faut donc faire deux fois les tests avec dans un cas php 7.3 puis dans l'autre php 8.2, soit je serai plutôt partisan de conserver php 7.3 à cet endroit pour détecter au plus vite les incompatibilités que nous n'aurions pas détecté avec nos environnements de tests en php 8.2
extensions: mysqli
tools: composer:v2
env:
Expand Down Expand Up @@ -105,13 +103,11 @@ jobs:
-F "admin_email=test@example.com" \
-F "submit=Continue" \
"http://localhost/?PagePrincipale&installAction=install"
- name: Set wakka.config.php writable
run: |
sudo chown www-data:www-data ${{ github.workspace }}/wakka.config.php
sudo chmod 0777 ${{ github.workspace }}/wakka.config.php
- name: Run test suite
run: composer test


6 changes: 2 additions & 4 deletions Dockerfile
View file
@@ -1,4 +1,4 @@
FROM lavoweb/php-7.3:composer
FROM lavoweb/php-8.2:composer

This comment has been minimized.

Sign in to view

Copy link
@J9rem

J9rem Sep 8, 2023

Contributor

à cet endroit, je pense que nous pouvons garder php 8.2 car c'est la config pour une installation docker et ce fichier ne semble pas utilisé par le workflow Github pour faire les tests.

Effectivement, c'est une très bonne idée de le passer en 8.2 ici

# Add MySQLi
RUN docker-php-ext-install mysqli
Expand All @@ -19,8 +19,6 @@ RUN mkdir -p themes/margot \
RUN curl -sS https://getcomposer.org/installer | php -- --install-dir=/usr/local/bin --filename=composer

# Node & NPM & Yarn
RUN curl -sL https://deb.nodesource.com/setup_14.x | bash -
RUN curl -sL https://deb.nodesource.com/setup_18.x | bash -
RUN apt-get install -y --no-install-recommends nodejs
RUN curl -L https://npmjs.org/install.sh | sh
RUN npm install -g yarn

1 comment on commit ee9316f

@mrflos
Copy link
Contributor Author

@mrflos mrflos commented on ee9316f Sep 9, 2023 •
edited

Choose a reason for hiding this comment

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

Ok merci du retour @J9rem , je comprend mieux pourquoi cette vieille version, et du coup j'ai préféré dupliquer les tests pour s'assurer de la compatibilité minimale avec php 7.3 et de celle utilisée pour l'image docker (avec php 8.2), comme ca on couvre le spectre entier des versions supportées.

Please sign in to comment.