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

Ensuring the xip extraction will work #167

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

Conversation

aln787
Copy link

@aln787 aln787 commented Sep 14, 2016

- Adding an alert if the os version is less than 10.11.5. Issue #163

Copy link
Member

@KrauseFx KrauseFx left a comment

Choose a reason for hiding this comment

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

Thanks for the pull request, I added some comments before it's ready

@@ -48,6 +48,18 @@ def current_symlink
File.symlink?(SYMLINK_PATH) ? SYMLINK_PATH : nil
end

def os_version_compatibility_issue?(version)
Copy link
Member

Choose a reason for hiding this comment

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

Could you add some comments here with context on why this is necessary?

Copy link
Author

Choose a reason for hiding this comment

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

I will add a few comments describing the issue with<10.11.5 failing to extract the archive

Copy link
Author

Choose a reason for hiding this comment

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

I added a comment. Let me know If you would like me to rebase and submit as a single commit.

@@ -35,6 +35,7 @@ def validate!
super

help! 'A VERSION argument is required.' unless @version
fail Informative, 'An OS X version >10.11.4 is required for xcode 8.' if @installer.os_version_compatibility_issue?(@version)
Copy link
Member

Choose a reason for hiding this comment

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

Should be Xcode

Copy link
Author

@aln787 aln787 left a comment

Choose a reason for hiding this comment

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

I made the requested changes. Let me know If you would like me to rebase and submit as a single commit.

@@ -48,6 +48,18 @@ def current_symlink
File.symlink?(SYMLINK_PATH) ? SYMLINK_PATH : nil
end

def os_version_compatibility_issue?(version)
Copy link
Author

Choose a reason for hiding this comment

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

I added a comment. Let me know If you would like me to rebase and submit as a single commit.

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.

2 participants