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

feat: only use rollup output for build #3597

Merged
merged 1 commit into from
Dec 18, 2023

Conversation

chmelevskij
Copy link
Member

Current build setup copies files into folder and then runs full yarn installation.

Since we are building everything based on the usage we don't need all the node_modules and friends.

This changes reduces final package size after unpack on mac by ~100mb. Need to verify other platforms

Before

Screenshot 2023-10-08 at 20 26 58 Screenshot 2023-10-08 at 20 28 18

After

Screenshot 2023-10-08 at 20 25 56 Screenshot 2023-10-08 at 20 27 29

@sonarqubecloud
Copy link

sonarqubecloud bot commented Oct 8, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

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
31.7% 31.7% Duplication

idea Catch issues before they fail your Quality Gate with our IDE extension sonarlint SonarLint

@github-actions

This comment has been minimized.

@blckmn
Copy link
Member

blckmn commented Oct 8, 2023

AUTOMERGE: (FAIL)

  • github identifies PR as mergeable -> FAIL
  • 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 -> PASS
  • assigned to an approver -> PASS
  • approver count at least three -> FAIL

@nerdCopter
Copy link
Member

nerdCopter commented Oct 9, 2023

"debug" version - Debian Linux 11

before/master:

$ du -h release/betaflight-configurator*
202M	release/betaflight-configurator-10.10.0_debug_d042471d-1.x86_64.rpm
136M	release/betaflight-configurator_10.10.0-debug-d042471d_amd64.deb
206M	release/betaflight-configurator_10.10.0-debug-d042471d_linux64-portable.zip

installed: 655M ./betaflight-configurator/

after/pr-3597:

$ du -h release/betaflight-configurator*
177M	release/betaflight-configurator-10.10.0_debug_55b0aaf9-1.x86_64.rpm
126M	release/betaflight-configurator_10.10.0-debug-55b0aaf9_amd64.deb
178M	release/betaflight-configurator_10.10.0-debug-55b0aaf9_linux64-portable.zip

installed: 539M ./betaflight-configurator/

oddly, all the *.js in the root installation folder, not the ./js/tabs/ (with exception of receiver_msp.js)

problems found:
flash-tab: load-online, no flash-button not enabled. same for local-local
nothing else found (shallow testing)

@ctzsnooze
Copy link
Member

ctzsnooze commented Oct 9, 2023

Thanks!

There is a small, long-term problem (not just from this PR) with the content of the Mac '.dmg' file.

If I download the Build for macOS, I get a file Betaflight-Configurator-macOS.zip (size 150.1Mb).

Double-click this -> betaflight-configurator_10.10.0-debug-daf3709_macOS.dmg (size 150.6Mb)

Double-click this -> disk image opens normally.

Is it possible to distribute the Mac version not as a zip of the dmg, but as the dmg itself? ie, at the stage of zipping the .dmg into a zip archive, omit that step for the Mac version?

Normally, Mac applications are distributed as dmg's only.

@ctzsnooze
Copy link
Member

ctzsnooze commented Oct 10, 2023

When testing this PR, using macOSX, all the javascript files for Tabs, ie all those from /js/tabs, except for receiver_msp.js and receiver_msp.map, have been moved to the root directory. Apart from the receiver_msp files, /js/tabs/ is empty.

The tabs still work properly.

@nerdCopter
Copy link
Member

@ctzsnooze , did your flashing tab allow for flashing? mine did not in this PR.

@nerdCopter
Copy link
Member

nerdCopter commented Oct 10, 2023

Old versions of Configurator retained the .js files inside the ./js/tabs/ folder. unsure when or why this changed. Would prefer such.

@chmelevskij
Copy link
Member Author

Old versions of Configurator rtained the .js files inside the ./js/tabs/ folder. unsure when or why this changed. Would prefer such.

What is the value in that? We don't include the source for C code why should we do that for configurator?

I could adjust the rollup to keep that structure if we really do need it.

@nerdCopter
Copy link
Member

so the packaged configurator does not run plain javascript? how does debug/inspect work? i see plaintext javascript in it.

@chmelevskij
Copy link
Member Author

It does, but it's bundled based on usage (import/export) not just on the fact that we have things in node_modules

I will look into slightly different builds for prod and debug. I don't think you can even open dev tools in production builds.

We can see the original structure of the files etc. because of the sourcemaps (those .map files in final output). Chrome then re-assembles back the file structure in devtools. It has nothing to do with the original files being there.

@haslinghuis
Copy link
Member

@chmelevskij please have a look - as Flash Firmware is not available after loading firmware.

@chmelevskij
Copy link
Member Author

Hey there, will try to find time for this today 🙈

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

@haslinghuis haslinghuis removed this from the 10.10.0 milestone Dec 5, 2023
@chmelevskij chmelevskij force-pushed the feat/reduce-build-output branch from 7761de5 to fa08d35 Compare December 10, 2023 17:02

This comment has been minimized.

@chmelevskij
Copy link
Member Author

@haslinghuis, had to copy in worker script we use in flasher tab https://github.com/betaflight/betaflight-configurator/blob/master/src/js/workers/hex_parser.js seemed to work now. Please verify

@haslinghuis
Copy link
Member

git clean -xdf - so have to rebuild :)

This comment has been minimized.

@haslinghuis
Copy link
Member

haslinghuis commented Dec 10, 2023

image

image

image

image

@chmelevskij chmelevskij force-pushed the feat/reduce-build-output branch from fbcd67d to 5f4716e Compare December 17, 2023 09:13
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions

15.5% Duplication on New Code (required ≤ 3%)

See analysis details on SonarCloud

@haslinghuis haslinghuis added this to the 10.10.0 milestone Dec 17, 2023
Copy link
Contributor

Do you want to test this code? Here you have an automated build:
Betaflight-Configurator-Android
Betaflight-Configurator-Linux
Betaflight-Configurator-macOS
Betaflight-Configurator-Windows
WARNING: It may be unstable and result in corrupted configurations or data loss. Use only for testing!

Copy link
Member

@nerdCopter nerdCopter left a comment

Choose a reason for hiding this comment

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

  • approving, briefly tested both vite and nwjs

@haslinghuis haslinghuis merged commit 8f85f08 into betaflight:master Dec 18, 2023
7 of 8 checks passed
chmelevskij added a commit to chmelevskij/betaflight-configurator that referenced this pull request Apr 27, 2024
chore: reduce final package build output
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: COMPLETED
Development

Successfully merging this pull request may close these issues.

5 participants