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.

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

Jump to bottom

Multijobs travis builds #47

Closed
wants to merge 5 commits into from
Closed

Multijobs travis builds #47

wants to merge 5 commits into from

Conversation

alaxalves
Copy link
Member

No description provided.

@alaxalves alaxalves force-pushed the feat/multistage-build branch 2 times, most recently from f167829 to a6202b8 Compare April 14, 2020 02:37
@alaxalves alaxalves force-pushed the feat/multistage-build branch 15 times, most recently from 7b8174f to 568d74a Compare April 14, 2020 04:31
@alaxalves alaxalves changed the title WIP: Multistage docker and travis builds Multistage docker and travis builds Apr 14, 2020
alaxalves
alaxalves commented Apr 14, 2020
View reviewed changes
Gemfile Show resolved Hide resolved
@jywarren
Copy link
Member

Actually I think I need to rely on @icarito for this one. Thanks!!

icarito
icarito reviewed Apr 14, 2020
View reviewed changes
Copy link
Member

@icarito icarito left a comment

Choose a reason for hiding this comment

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

If the benefit is speed of travis runs, we would need to balance with Dockerfile complexity.

Dockerfile Show resolved Hide resolved
scripts/install-deps.sh Outdated Show resolved Hide resolved
icarito
icarito reviewed Apr 14, 2020
View reviewed changes
Dockerfile Outdated Show resolved Hide resolved
@alaxalves alaxalves changed the title Multistage docker and travis builds Multijobs travis builds Apr 14, 2020
@alaxalves alaxalves force-pushed the feat/multistage-build branch 3 times, most recently from 4606af3 to 10330c0 Compare April 14, 2020 23:20
alaxalves
alaxalves commented Apr 14, 2020
View reviewed changes
Dockerfile Show resolved Hide resolved
alaxalves
alaxalves commented Apr 14, 2020
View reviewed changes
Gemfile Outdated Show resolved Hide resolved
alaxalves
alaxalves commented Apr 14, 2020
View reviewed changes
scripts/install-deps.sh Outdated
apt-get install -y \
gdal-bin \
python-gdal \
pandoc \
Copy link
Member Author

Choose a reason for hiding this comment

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

Had to include this since:
image

Copy link
Member

@icarito icarito May 21, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Hi,
In second review I still see that you've introduced pandoc as dependency. In previous version kramdown did the exact same and pandoc is just an optional dependency (which is very large). Please try again without including pandoc, I took care that it wasn't required, unless I am wrong. Thanks for the perseverance!

Suggested change
pandoc \

Copy link
Member Author

Choose a reason for hiding this comment

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

@icarito When I remove pandoc I get the error:
image

Via: https://travis-ci.com/github/publiclab/mapknitter-exporter-sinatra/jobs/345621753

Copy link
Member Author

Choose a reason for hiding this comment

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

Should I revert 3428c40 to get this merged?

@alaxalves alaxalves mentioned this pull request May 25, 2020
Closed
@alaxalves alaxalves mentioned this pull request Jun 7, 2020
Closed
@icarito icarito mentioned this pull request Jun 7, 2020
Merged
icarito
icarito reviewed Jun 8, 2020
View reviewed changes
Copy link
Member

@icarito icarito left a comment

Choose a reason for hiding this comment

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

I've just sent you a pull request to this branch that resolves the pandoc dependency. Thanks for your hard work!

Gemfile Outdated Show resolved Hide resolved
@alaxalves
Copy link
Member Author

Closing this since #58

@alaxalves alaxalves closed this Jun 9, 2020
@alaxalves alaxalves deleted the feat/multistage-build branch June 9, 2020 00:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@icarito icarito icarito left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@alaxalves @jywarren @icarito