search

NordicSemiconductor / pc-nrfconnect-shared

Dependency management for nRF Connect for Desktop packages
Other
22 stars 8 forks source link

feat: export all package.json listed nrfutil modules from shared #861

Closed boundlesscalm closed 10 months ago

boundlesscalm commented 10 months ago

This PR does a few things, all of which are required to allow shared to log the nrfutil module version which are in use.

  1. All modules listed in the package.json of an app are accessible from shared and are lazy-loaded upon the first get
  2. To avoid cyclic dependencies and for seperation of concern, the types of nrfutil and nrfutil device were split into separate index.ts files
  3. Since all nrfutil modules are available in shared, enabling verbose logging now works for all modules (compared to only nrfutil device like previously). Depends on https://github.com/NordicSemiconductor/pc-nrfconnect-shared/pull/860.
datenreisender commented 10 months ago

Since this depends on https://github.com/NordicSemiconductor/pc-nrfconnect-shared/pull/860 (as you wrote) I would usually not base this PR on main but instead on the branch feat/persisted-verbose-logging of that PR.

Or do you prefer to keep PRs against main since this is where the code will end up?

For me basing this PR on feat/persisted-verbose-logging would make reviewing it independent of that other PR a bit easier, but I can achieve the same by looking at the diff manually at https://github.com/NordicSemiconductor/pc-nrfconnect-shared/compare/feat/persisted-verbose-logging...feat/shared-controlled-nrfutil-modules

At the end it doesn't change a lot, as soon as #860 is merged, the situation will be the same:

boundlesscalm commented 10 months ago

I wasn't aware that the PR would automatically be redirected against main, that makes this so much nicer. I'll re-target this against the verbose logging branch. Thanks!

boundlesscalm commented 10 months ago

@datenreisender Thanks for the suggestions! Implemented all of them and merged your branch.