search

Fitbit / fitbit-sdk-toolchain

Toolchain for building Fitbit apps
https://dev.fitbit.com
BSD 3-Clause "New" or "Revised" License
52 stars 7 forks source link

Add C api version for native components #318

Closed catalinamocanu closed 1 year ago

catalinamocanu commented 1 year ago

Summary of Changes

For native components read the C api version information from the bundle similarly to what we do for "platform" information. The C api version is specific to each bundle.

If the bundle doesn't contain a C api version, then return "N/A" and skip this information in the manifest file. Otherwise use the C api version to populate the manifest inside appPackageManifest.

Description of testing

Checked the changes are backwards compatible by generating a manifest for a native component without C api version. Checked that the manifest contains the C api version information if it is present in the native component.

NOTE: I haven't yet changed the tests as I would like to make sure the current implementation is the way to go and them I'll fix the existing tests and create new ones.

catalinamocanu commented 1 year ago

This is missing tests update, but the code changes are ready. Please let me know if the changes are OK so I can move on to updating the tests.

Hexxeh commented 1 year ago

Looks good to me, but please add tests before merge :)

coveralls commented 1 year ago

Coverage Status

coverage: 79.958% (+1.8%) from 78.12% when pulling e73c2ddfdbc04255ec964183063a38814d12fb61 on cmocanu/add_c_api_version_to_native_components into ce21112452aca67131bc49c05e2f7546f30d08ad on master.

catalinamocanu commented 1 year ago

@Hexxeh @gmaniak finally managed to add the unit tests for nativeComponents and all tests pass.

Please review the PR. Thanks!!

Hexxeh commented 1 year ago

LGTM