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

Feature/fix build deps #28

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

danielt3
Copy link

This adds instructions about how to install build dependencies in Debian 10 systems. Please, review and make your comments.

Thank you.

Copy link
Member

@Semphriss Semphriss left a comment

Choose a reason for hiding this comment

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

  • Some libraries (such as SDL2 and libcurl4 are missing, and the sudo apt install's are redundant. Please check this line for reference: sudo apt-get update && sudo apt-get install -y cmake build-essential libogg-dev libvorbis-dev libopenal-dev libboost-all-dev libsdl2-dev libsdl2-image-dev libfreetype6-dev libraqm-dev libcurl4-openssl-dev libglew-dev libharfbuzz-dev libfribidi-dev
  • Please don't make an entire section just for that; put it at the end of the preceeding section instead, with an introduction like "For ease of use, here are some installation lines for some Linux distributions:" and if the line given above works without edit, please specify it works for both Debian and Ubuntu.

@danielt3
Copy link
Author

Thank you for your help and comments. Answers inline.

* [ ]  Some libraries (such as SDL2 and libcurl4 are missing, and the `sudo apt install`'s are redundant. Please check this line for reference: `sudo apt-get update && sudo apt-get install -y cmake build-essential libogg-dev libvorbis-dev libopenal-dev libboost-all-dev libsdl2-dev libsdl2-image-dev libfreetype6-dev libraqm-dev libcurl4-openssl-dev libglew-dev libharfbuzz-dev libfribidi-dev`

Thank you for your comments. I will add as one single line as you specified. Also, I don't recommend using -y on other people's machines. Can I remove it?

* [ ]  Please don't make an entire section just for that; put it at the end of the preceeding section instead, with an introduction like "For ease of use, here are some installation lines for some Linux distributions:" and if the line given above works without edit, please specify it works for both Debian and Ubuntu.

OK, will do so for the section. For specifying it works for Debian and Ubuntu, I don't own an Ubuntu machine and I didn't tested it running in Ubuntu. This is why I didn't mention it. Can I just put a message like: "These were not tested in Ubuntu but they are likely to work."?

@tobbi
Copy link
Member

tobbi commented May 13, 2021

Is this good to go?

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