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

Add some options for bundle when create the installer on windows #224

Merged
merged 1 commit into from
May 18, 2015

Conversation

MikeLing
Copy link
Contributor

No description provided.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 77.64% when pulling a8fc2d3 on MikeLing:Bugfix-1161542 into 240e25d on mozilla:master.

@@ -80,14 +80,21 @@ def do_bundle(options):
if os.path.isdir(dirname):
shutil.rmtree(dirname)
# create a intaller
if IS_WIN:
Copy link
Contributor

Choose a reason for hiding this comment

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

We don't need the IS_WIN check yet - for now this part of the script is windows only (we will make it work for linux with the next bug). So please remove it for now. Also, the default values for python_path and nsis_path can be defined as a default= argument in the add_argument call, so you can just remove all this block.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm, I see. But when we call bundle under linux, the default argument will be also given to bundle if we forget to give a python path. So, why don't we set different default value of path when we call different bundle funtion?

Copy link
Contributor

Choose a reason for hiding this comment

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

Well, we will! But it is for the next patch - for now, the bundle command is only used in windows, so let keep things simple.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I mean you can treat this patch as a windows bundle function.

python_path = 'C:\Python27'
nsis_path = 'C:\NSIS'

And, we when bundle be called under Linux system.

python_path = /usr/lib/python2.7

Copy link
Contributor

Choose a reason for hiding this comment

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

The goal of this patch is to remove the environment variables used to use command line arguments instead. Let's just do this for now, and not something else - it is easy to do, and easy to review. Let's talk on irc about that!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see. :)

Sorry, I have some classes this afternoon. I will send you email after that. Let's fix this issue first. :D

@MikeLing MikeLing force-pushed the Bugfix-1161542 branch 2 times, most recently from 8e03b49 to 893db77 Compare May 18, 2015 11:25
@coveralls
Copy link

Coverage Status

Coverage remained the same at 80.45% when pulling 893db77 on MikeLing:Bugfix-1161542 into 0919e71 on mozilla:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 80.45% when pulling 214beca on MikeLing:Bugfix-1161542 into 0919e71 on mozilla:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 80.45% when pulling 64b949c on MikeLing:Bugfix-1161542 into 0919e71 on mozilla:master.

@MikeLing MikeLing force-pushed the Bugfix-1161542 branch 2 times, most recently from f0f3ef8 to 80717e2 Compare May 18, 2015 13:30
@coveralls
Copy link

Coverage Status

Coverage remained the same at 80.45% when pulling db6c0cf on MikeLing:Bugfix-1161542 into 0919e71 on mozilla:master.

parkouss added a commit that referenced this pull request May 18, 2015
Add some options for bundle when create the installer on windows
@parkouss parkouss merged commit 341f93d into mozilla:master May 18, 2015
@MikeLing MikeLing deleted the Bugfix-1161542 branch May 19, 2015 00:19
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