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

Universal icon usage and allow for license check. #143

Merged
merged 7 commits into from Feb 4, 2018

Conversation

@ntoll
Copy link
Contributor

@ntoll ntoll commented Jan 29, 2018

This PR introduces three updates:

  1. If an icon is specified it will be used in the install/uninstall process and as the icon for the installer itself.
  2. If a license path is given in the Application section of the configuration file an additional step will take place before installation to check the user's agreement to abide by the displayed license. If the license is not given, the extra step is omitted (the default behaviour).
  3. Documentation updates to reflect the above two changes.
ntoll added 2 commits Jan 29, 2018
…icense is specified, include a step in the installer to ask for agreement to the terms of the referenced license.
Copy link
Owner

@takluyver takluyver left a comment

This looks good. For bonus points, would you like to add a couple of bullet points about the changes to the release notes for 2.1? :-)

@@ -80,6 +81,13 @@ Application section
If you use the Python API, this parameter can also be passed as a file-like
object, such as :class:`io.StringIO`.

.. describe:: license (optional)

This comment has been minimized.

@takluyver

takluyver Jan 29, 2018
Owner

For the sake of explicitness, I like your earlier suggestion to call this license_file - that makes it clear that it's a path, not just a name like GPL.

@@ -282,7 +283,7 @@ def prepare_shortcuts(self):
copy to the build directory. Prepare target and parameters for these
shortcuts.
Also copies shortcut icons
Also copies shortcut icons, and, if specified, the license file.

This comment has been minimized.

@takluyver

takluyver Jan 29, 2018
Owner

It doesn't make sense to me to add this to prepare_shortcuts(), but I don't think any of the other methods are right either. Let's add a new copy_license() method for this.

@takluyver
Copy link
Owner

@takluyver takluyver commented Jan 29, 2018

ntoll added 4 commits Jan 30, 2018
@ntoll
Copy link
Contributor Author

@ntoll ntoll commented Jan 30, 2018

@takluyver OK... all done. I've bumped it to version 2.2 since 2.1 is already out. Happy to change anything else if need be.

@ntoll
Copy link
Contributor Author

@ntoll ntoll commented Feb 4, 2018

@takluyver hey hey... any reasons this hasn't been merged yet..? Would like to use these updates in my automated build in appveyor (i.e. I can pip install pynsist with these changes). :-)

@takluyver
Copy link
Owner

@takluyver takluyver commented Feb 4, 2018

No, I just got busy with other things and forgot about it. Pressing buttons now...

@takluyver takluyver merged commit 22401a4 into takluyver:master Feb 4, 2018
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@takluyver
Copy link
Owner

@takluyver takluyver commented Feb 4, 2018

OK, 2.1 is now out. :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants