Skip to content

Enhance add-on installation messages to display version information#7542

Merged
feerrenrut merged 2 commits into
nvaccess:masterfrom
BabbageCom:t5324
Oct 2, 2017
Merged

Enhance add-on installation messages to display version information#7542
feerrenrut merged 2 commits into
nvaccess:masterfrom
BabbageCom:t5324

Conversation

@LeonarddeR

@LeonarddeR LeonarddeR commented Aug 30, 2017

Copy link
Copy Markdown
Collaborator

Link to issue number:

fixes #5324

Summary of the issue:

When updating add-ons, it is not yet clear from which version to which version you're updating.

Description of how this pull request fixes the issue:

  1. Show the old version and new version in the message that states that you are updating a currently installed add-on.
  2. Show an alternative message when you're trying to update an add-on to the same version
  3. In these confirmation messages, also show the summary (readable name) of the add-on.

Testing performed:

Updated add-ons to newer and same versions, the messages did show up as expected.

Known issues with pull request:

None I'm aware of

Credits

Many thanks to @beqabeqa473 for the initial work.

@LeonarddeR LeonarddeR requested a review from feerrenrut August 30, 2017 06:09
@josephsl

josephsl commented Aug 30, 2017 via email

Copy link
Copy Markdown
Contributor

@LeonarddeR LeonarddeR changed the title Enhance add-on installation messages to display version informatione Enhance add-on installation messages to display version information Aug 30, 2017
Comment thread source/gui/addonGui.py
return
summary=bundle.manifest["summary"]
curVersion=prevAddon.manifest["version"]
newVersion=bundle.manifest["version"]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Are we guaranteed that both the old and new addon will have a manifest version parameter? If not what would happen?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

It wasn't yet possible to install add-ons without a version, and this is not handled gracefully (i.e. an error is raised in the log and there is no visible error). Theoretically, this code indeed would raise an error if the old add-on doesn't have a version in its manifest, but it shouldn't have been possible to install it before.

Having said that, I think it would be good to have a visible error raised if either the old or the new manifest is broken.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is the error for a broken manifest something that you intend to resolve in this PR? I would suggest not.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Nope, I"m not intending to do this as it is only partially related to this pr.

@feerrenrut feerrenrut self-requested a review August 30, 2017 09:42
feerrenrut added a commit that referenced this pull request Sep 11, 2017
Merge remote-tracking branch 'origin/pr/7542' into next
@feerrenrut feerrenrut merged commit 89681b9 into nvaccess:master Oct 2, 2017
@nvaccessAuto nvaccessAuto added this to the 2017.4 milestone Oct 2, 2017
feerrenrut added a commit that referenced this pull request Oct 2, 2017
@LeonarddeR LeonarddeR added the BabbageWork Pull requests filed on behalf of Babbage B.V. label Oct 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

BabbageWork Pull requests filed on behalf of Babbage B.V.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Report current version on addon updates

5 participants