Skip to content

Conversation

OkanPinar
Copy link
Contributor

Addition dev container support can be good feature for developers who don't want to install c++ related build tools.

PS: CTEST did not work without change directory to build.

@lemire
Copy link
Member

lemire commented Mar 2, 2023

That's not bad at all.

Copy link
Member

@anonrig anonrig left a comment

Choose a reason for hiding this comment

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

Other than, 4 indentation I'm OK with this, if there is another widely known IDE (other than VSCode) that supports DevContainers. I'm not in favor of introducing IDE specific features to the codebase.

It seems that Jetbrains does not support dev containers (referencing: https://youtrack.jetbrains.com/issue/IDEA-292050).

Do you have more knowledge on this @OkanPinar?

@OkanPinar
Copy link
Contributor Author

OkanPinar commented Mar 3, 2023

It seems that there is no direct support for the same feature. @anonrig What about using docker directly to build and project like the following:
Docker file:

FROM debian:11-slim

RUN apt-get update && apt-get install -y \
    apt-transport-https \
    gcc \
    clang \
    clang-tools \
    cmake \
    libicu-dev

WORKDIR /repo

Docker-compose:

version: '3.9'

services:
  ada-builder:
    build: .
    entrypoint: ['bash', '-c', 'cmake -B build && cmake --build build && cd build && ctest --output-on-failure']
    volumes:
      - .:/repo

@anonrig
Copy link
Member

anonrig commented Mar 3, 2023

How can we remove the need of a docker-compose file? Can we achieve the same thing with Dockerfile?

@OkanPinar
Copy link
Contributor Author

It seems working like following:

Dockerfile:

FROM debian:11-slim

RUN apt-get update && apt-get install -y \
    apt-transport-https \
    gcc \
    clang \
    clang-tools \
    cmake \
    libicu-dev

WORKDIR /repo

CMD ["bash", "-c", "cmake -B build && cmake --build build && cd build && ctest --output-on-failure"]

Docker Commands:

docker build -t ada-builder .
docker run --rm -it -v ${PWD}:/repo ada-builder

@anonrig
Copy link
Member

anonrig commented Mar 3, 2023

Can you add dockerfile to the root, and add a part to README with the command to run dockerfile?

@anonrig anonrig requested a review from lemire March 3, 2023 15:46
@anonrig anonrig merged commit c10cac9 into ada-url:main Mar 8, 2023
@anonrig
Copy link
Member

anonrig commented Mar 8, 2023

Thank you for your contribution @OkanPinar

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants