Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Build Prototype Monkey Island AppImage #1069
Build Prototype Monkey Island AppImage #1069
Changes from all commits
12c40c3
0230c26
1d73f6e
986219b
4b5415a
98b64da
a057dec
dd9e4bd
ea14bcc
fc2f8ec
fef44bc
4cb28db
e8bb2e6
8b37038
74e0dfd
5b781c5
e6bf085
1f57610
21e0b51
ef1ef34
64018eb
2d971d9
438a270
3f6c268
d265238
a09cd8f
044c656
115368f
fdeec3a
45367bb
29c9c72
20a3d31
e1209dc
b5e8d89
1fad6b4
921c4d0
1ac67cf
eae5881
3c113f7
a97bd19
7910d9b
5e56257
412aa2b
b0af8b1
2c75eab
3d938f2
05a368e
8278e0e
5b1296e
bbe075b
ed3d55c
f7cc018
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Woulnd't it be easier in terms of integration testing, to NOT require a new machine each time?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LXCs are very quick to spin up. So are VMs in the right environment. Regardless, the README contains instructions on how to clean up after the script runs so that you don't need to use a fresh environment each time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you move links like these to the top of the file, it will be easier to fix once they get broken, since you won't have to scroll the whole file. Not worth holding up the PR though
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like having it localized, as that function is the only place it's used, but I see your point. I'll leave it for now, as this may get reworked anyway in version 2.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe add
sudo apt update
as well?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The VM should be upgraded before building. I'll add details in a readme.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even if it's docker or something, how do you know that the repositories will be updated? I'd rather we have
sudo apt update
in scripts than specify in the readme that "you need to run sudo apt update".