Skip to content

Conversation

@haslinghuis
Copy link
Member

@TheIsotopes
Copy link
Contributor

@haslinghuis for the portable version i would prefer a version with nwBuilderOptions zip: false,. Is that doable?

McGiverGim
McGiverGim previously approved these changes Jan 28, 2022
@haslinghuis
Copy link
Member Author

@TheIsotopes done. Thanks for the tip (as configurator is doing the same).

@McGiverGim
Copy link
Member

I don't think we must use different parameters, we can't discuss to change it for all, but not only for portable.

@haslinghuis
Copy link
Member Author

@McGiverGim configurator is using the same parameter. Otherwise happy to remove.

@TheIsotopes
Copy link
Contributor

@McGiverGim as it is set now, it works on all versions. it should stay that way, it's the same as with the configurator.

@McGiverGim
Copy link
Member

Long story short... if I remember correctly, in the Configurator was put false because for some user the startup time was very high (maybe some antivirus?). It has a drawback: the installer take a lot more because it registers any file that install.

betaflight/betaflight-configurator#1057

The only problem is that the UPDATE of the app produced all the problems we suffered in the Configurator. I hope that with the new installer this does not happen anymore.

I prefer the compressed way, but if we want to use it uncompressed, is not a problem to me neither. But then we need to "uninstall" the Blackbox when installing to be sure to not let a lot of old files from different versions. Like we do in the Configurator. The installer only writes the new files, it does not remove the old one. When is compressed is not a problem, the files are the same, but now it will not.

@haslinghuis
Copy link
Member Author

haslinghuis commented Jan 28, 2022

According to the linked issue nwBuilderOptions zip: true should work on both clients now???

@McGiverGim I'd like to be consistent: should we change this parameter on configurator (to true) also?
@TheIsotopes do you experience any issue with zip: true?

@TheIsotopes
Copy link
Contributor

TheIsotopes commented Jan 28, 2022

do you experience any issue with zip: true?

@haslinghuis Actually no, but loading takes longer on older computers.
That's why i prefer nwBuilderOptions zip: false only on portable version,
All you have to do is unzip it, no matter where. No more annoying installation necessary and it just starts faster.

@haslinghuis
Copy link
Member Author

@McGiverGim It's your take. I'm happy either way,

@haslinghuis haslinghuis self-assigned this Jan 29, 2022
@asizon
Copy link
Member

asizon commented Jan 29, 2022

These changes are not aplying for mac users, should we aplied also for this SO?

gulpfile.js Outdated
macIcns: './images/bf_icon.icns',
macPlist: { 'CFBundleDisplayName': 'Betaflight Blackbox Explorer'},
winIco: './images/bf_icon.ico',
zip: false,
Copy link
Member

Choose a reason for hiding this comment

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

If we want to go with this, we need to do mcgiver requested changes on the installer side.

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@McGiverGim
Copy link
Member

My comment was, as Asizon said, because if we add the parameter, we need to modify the installer to uninstall the earlier before installing. A lot of garbage can remain if we don't do that. If we add that, no problem to use it uncompressed.

@haslinghuis haslinghuis requested a review from asizon January 29, 2022 13:23
@blckmn
Copy link
Member

blckmn commented Jan 30, 2022

AUTOMERGE: (FAIL)

  • github identifies PR as mergeable -> PASS
  • assigned to a milestone -> PASS
  • cooling off period lapsed -> PASS
  • commit count less or equal to three -> PASS
  • Don't merge label NOT found -> PASS
  • at least one RN: label found -> FAIL
  • Tested label found -> FAIL
  • assigned to an approver -> PASS
  • approver count at least three -> FAIL

@haslinghuis haslinghuis merged commit 7c88a01 into betaflight:master Jan 30, 2022
@haslinghuis haslinghuis deleted the add_portable_windows branch January 30, 2022 14:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Newer builds of 3.6 Not Win7 Compatible

5 participants