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

Use well known location for bundled english content pack #441

Merged
merged 6 commits into from
Mar 30, 2017
Merged

Use well known location for bundled english content pack #441

merged 6 commits into from
Mar 30, 2017

Conversation

mrpau-richard
Copy link
Contributor

Summary

Use well known location for bundled english content pack.
Fix build script, step arrangements.

Issues addressed

Fixes #438

/cc @cpauya @mrpau-eduard

@cpauya
Copy link
Contributor

cpauya commented Feb 28, 2017

@mrpau-richard I still see a comment about Pyrun at https://github.com/learningequality/ka-lite-installers/pull/441/files#diff-632e66fabe1d6056f28c4739c768f4afL212 - can you clear that up?

@mrpau-richard
Copy link
Contributor Author

@cpauya OK I will remove it.

cpauya
cpauya reviewed Feb 28, 2017
View reviewed changes
Copy link
Contributor

@cpauya cpauya left a comment •
edited
Loading

Choose a reason for hiding this comment

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

@mrpau-richard The step to clone the create-dmg repo must also be done early - it's a requirement during the build thus it's good to check if it's available or not early on.

Here's the code I'm talking about where it clones the repo during the last step of the build process: https://github.com/mrpau/installers/blob/8f3cc1490eb1197d9287901f9d7500d4b31a68e2/osx/build.sh#L440

Overall, it's good to see your updates! 😸

osx/build.sh Outdated
@@ -56,6 +56,7 @@ STEPS=15

# TODO(cpauya): get version from `ka-lite/kalite/version.py`
# Set the default value to `develop` as suggested by [@benjaoming](https://github.com/learningequality/ka-lite-installers/pull/433#discussion_r96399812), so we can use the VERSION environment in bamboo settings.
VERSION="0.17.x"

This comment was marked as spam.

Sign in to view

osx/build.sh

# Download python installer.
((STEP++))
PYTHON_DOWNLOAD_URL="https://www.python.org/ftp/python/2.7.12/python-2.7.12-macosx10.6.pkg"

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

osx/build.sh Outdated
((STEP++))
PYTHON_DOWNLOAD_URL="https://www.python.org/ftp/python/2.7.12/python-2.7.12-macosx10.6.pkg"
cd $TEMP_OUTPUT_PATH
wget --retry-connrefused --read-timeout=20 --waitretry=1 -t 100 --continue $PYTHON_DOWNLOAD_URL

This comment was marked as spam.

Sign in to view

osx/build.sh Outdated
@@ -56,6 +56,7 @@ STEPS=15

# TODO(cpauya): get version from `ka-lite/kalite/version.py`
# Set the default value to `develop` as suggested by [@benjaoming](https://github.com/learningequality/ka-lite-installers/pull/433#discussion_r96399812), so we can use the VERSION environment in bamboo settings.
VERSION="0.17.x"
VERSION=${VERSION:-"develop"}

This comment was marked as spam.

Sign in to view

cpauya
cpauya suggested changes Feb 28, 2017
View reviewed changes
Copy link
Contributor

@cpauya cpauya left a comment

Choose a reason for hiding this comment

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

Sorry, I selected Comment earlier - now I am requesting changes @mrpau-richard 😸

@mrpau-richard
Copy link
Contributor Author

@cpauya I've fixed all of your comments. Can you check it again? Thanks

cpauya
cpauya suggested changes Mar 1, 2017
View reviewed changes
Copy link
Contributor

@cpauya cpauya left a comment

Choose a reason for hiding this comment

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

Just a very minor request and we're good to go. 😸

osx/build.sh

test ! -d "$OUTPUT_PATH" && mkdir "$OUTPUT_PATH"

((STEP++))

This comment was marked as spam.

Sign in to view

@benjaoming benjaoming added the Mac label Mar 30, 2017
@benjaoming
Copy link
Contributor

Was this released for the 0.17.0 installer?

I think it looks like @cpauya 's last review comment has been addressed, so is it good to merge?

@cpauya
Copy link
Contributor

cpauya commented Mar 30, 2017

Oh sorry, yes @benjaoming this should be good to merge... doing that now.

@cpauya cpauya merged commit 33877ab into learningequality:develop Mar 30, 2017
@benjaoming
Copy link
Contributor

Great.. but just to be clear:

Was this released for the 0.17.0 installer?

@mrpau-richard
Copy link
Contributor Author

Hi @benjaoming yeah, it's for 0.17 released fixes.

@cpauya
Copy link
Contributor

cpauya commented Mar 31, 2017

But I don't see the changes on the develop branch - can you confirm @mrpau-richard if you have other branches with the fixes that we have released for 0.17.0?

@cpauya
Copy link
Contributor

cpauya commented Mar 31, 2017

Well now that I've pulled the recent changes... it's on the develop branch already since I merged it. But to be sure, the changes on this PR weren't included on the already released 0.17.0. Is that correct @mrpau-richard?

@mrpau-richard
Copy link
Contributor Author

mrpau-richard commented Mar 31, 2017 •
edited
Loading

@cpauya yeah, the changes of this PR was not in current 0.17 released.

@cpauya
Copy link
Contributor

cpauya commented Mar 31, 2017

Thanks for confirming.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cpauya cpauya cpauya requested changes

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

Successfully merging this pull request may close these issues.
3 participants
@mrpau-richard @cpauya @benjaoming