Skip to content

Gslux 635 use v4 release bundle #3180

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: integration-vue
Choose a base branch
from
Open

Gslux 635 use v4 release bundle #3180

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
29fd8d1
reference released bundle in v4 repo
mki-c2c Jul 30, 2024
21135b0
install cleanly via npm
mki-c2c Jul 30, 2024
fbc3f6c
update release reference to the bundle
mki-c2c Jul 30, 2024
283e3dc
use release url in package.json
mki-c2c Aug 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions geoportal/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ COPY geoportailv3_geoportal/static-ngeo /app/geoportailv3_geoportal/static-ngeo
RUN rm -rf /usr/lib/node_modules/ngeo
RUN mv /app/geoportailv3_geoportal/static-ngeo/ngeo /usr/lib/node_modules/ngeo

COPY . /app
RUN mkdir -p /app
COPY ./package.json /app

# jsapi generation
ADD ./jsapi /etc/apiv4/
Comment on lines 38 to 43
Copy link
Contributor

@tkohr tkohr Aug 6, 2024

Choose a reason for hiding this comment

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

I think we can get rid of the diff of the Dockerfile completely, as the npm install in between the two changes only concerns the JS API, if I see correctly. The package.json for the app is actually already copied in line 28.

Copy link
Contributor Author

@mki-c2c mki-c2c Aug 6, 2024

Choose a reason for hiding this comment

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

the main point of this diff is removing the COPY . /app and putting it after the jsapi build
but yes, we can remove

RUN mkdir -p /app
COPY ./package.json /app

Copy link
Contributor

@tkohr tkohr Aug 6, 2024

Choose a reason for hiding this comment

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

Ok I don't get the advantage of moving COPY . /app, neither as the WORKDIR changes, but I might be missing something.

Copy link
Contributor Author

@mki-c2c mki-c2c Aug 6, 2024

Choose a reason for hiding this comment

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

By moving the cp . as late as possible we avoid all unnecessary docker build steps (npm install for jsapi and build jsapi) if the source code changes
This is just a build optimization which may reduce the image build time if something changes in app

We can leave the Dockerfile as is if that is more comfortable for you.

Copy link
Contributor

@tkohr tkohr Aug 6, 2024

Choose a reason for hiding this comment

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

Alright, I'm fine with moving the COPY . /app

Expand All @@ -45,14 +46,14 @@ RUN node --version
RUN npm install --no-optional && npm cache clear --force
RUN /etc/apiv4/rebuild_api.sh

COPY . /app
WORKDIR /app
# sad fix, to allow webpack's file-loader to find files with query string & hash added
RUN ln -s /usr/lib/node_modules/@fortawesome/fontawesome-free/webfonts/fa-regular-400.eot /usr/lib/node_modules/@fortawesome/fontawesome-free/webfonts/fa-regular-400.eot?#iefix && \
ln -s /usr/lib/node_modules/@fortawesome/fontawesome-free/webfonts/fa-regular-400.svg /usr/lib/node_modules/@fortawesome/fontawesome-free/webfonts/fa-regular-400.svg#fontawesome && \
ln -s /usr/lib/node_modules/@fortawesome/fontawesome-free/webfonts/fa-solid-900.eot /usr/lib/node_modules/@fortawesome/fontawesome-free/webfonts/fa-solid-900.eot?#iefix && \
ln -s /usr/lib/node_modules/@fortawesome/fontawesome-free/webfonts/fa-solid-900.svg /usr/lib/node_modules/@fortawesome/fontawesome-free/webfonts/fa-solid-900.svg#fontawesome


# RUN make checks
RUN make build

Expand Down
2 changes: 1 addition & 1 deletion geoportal/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
"url": "https://github.com/Geoportail-Luxembourg/geoportailv3/issues"
},
"devDependencies": {
"luxembourg-geoportail": "https://github.com/Geoportail-Luxembourg/luxembourg-geoportail.git#942bd52874cf2144202ecde81e4a7ee64f0c8dd1",
"luxembourg-geoportail": "https://github.com/Geoportail-Luxembourg/luxembourg-geoportail/releases/download/GSLUX-635_create_release_CI_6606389/luxembourg-geoportail-lib-0.0.0-dev.tgz",
"@babel/core": "7.16.0",
"@babel/plugin-proposal-class-properties": "7.16.0",
"@babel/plugin-proposal-decorators": "7.16.0",
Expand Down