Skip to content
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.

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

util/docker-container: create docker container #3

Closed
wants to merge 1 commit into from

Conversation

ALEX11BR
Copy link

adds a dockerfile for this course's docker container

Prerequisite Checklist

  • Read the contribution guidelines regarding submitting new changes to the project;
  • Tested your changes against relevant architectures and platforms;
  • Updated relevant documentation (if needed).

Description of changes

Copy link
Contributor

@gabrielmocanu gabrielmocanu left a comment

Choose a reason for hiding this comment

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

I will add here some other things:

  1. Please move this intro a directory called hsi-runner instead of docker-container.
  2. Keep the same structure as here.
  3. You can copy the README file from there and change only URLs
  4. Add a Makefile, similar to here

Comment on lines 3 to 12
RUN apt-get update -y
RUN apt-get install -y gcc gcc-multilib nasm make build-essential
RUN apt-get install -y binutils
RUN apt-get install -y gdb
RUN apt-get install -y vim nano
RUN apt-get install -y tmate
RUN apt-get install -y python2.7
RUN apt-get install -y python3.10 python-is-python3

CMD [ "bash" ]
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
RUN apt-get update -y
RUN apt-get install -y gcc gcc-multilib nasm make build-essential
RUN apt-get install -y binutils
RUN apt-get install -y gdb
RUN apt-get install -y vim nano
RUN apt-get install -y tmate
RUN apt-get install -y python2.7
RUN apt-get install -y python3.10 python-is-python3
CMD [ "bash" ]
RUN apt-get update \
&& apt-get install -y \
gcc \
gcc-multilib \
nasm \
make \
build-essential \
binutils \
gdb \
vim \
nano \
tmate \
python2.7 \
python3.10 \
python-is-python3 \
&& apt-get clean \
&& rm -rf /var/lib/apt/lists/*
CMD [ "bash" ]

Copy link
Contributor

Choose a reason for hiding this comment

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

Here are some best practices about Dockerfile. We need to put all these commands inside a single one in order to have fewer layers.

@teodutu
Copy link
Member

teodutu commented Mar 13, 2024

Why are you adding this here? It will generate merge conflicts and not be available for the cs-pub-ro repo [1] until we sync them. I think it's best to do the development on our fork [1] and then periodically sync it [1] with this one via a PR bot such as [2].

[1] https://github.com/cs-pub-ro/hardware-software-interface/
[2] open-education-hub/operating-systems#350

Adds a Dockerfile and all the other necessary files for the `hsi-runner`.

Signed-off-by: Popa Ioan Alexandru <[email protected]>
@gabrielmocanu
Copy link
Contributor

OK, @ALEX11BR , I think @teodutu is right, sorry for confusion.
Please close this PR and open the same PR but on this repo: https://github.com/cs-pub-ro/hardware-software-interface/

@ALEX11BR ALEX11BR closed this Mar 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants