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: [sc-117417] Add semver support and device-os-output variable #7

Merged
merged 9 commits into from
Apr 6, 2023

Conversation

laupow
Copy link
Collaborator

@laupow laupow commented Apr 4, 2023

Story details: https://app.shortcut.com/particle/story/117417

Adds device-os-version output. When users pass a semver or latest as the device-os-version input, it may be useful to know which Device OS version the firmware was compiled against.

@laupow laupow changed the title feat: [sc-117417] Cellular canary fleet CI/CD pipeline feat: [sc-117417] Add device-os-output variable Apr 4, 2023
@wraithan
Copy link

wraithan commented Apr 4, 2023

Latest would be good to have, but I wanted latest LTS if it is available. What about enabling something like this tilde range syntax then we can just say ~2.2 which would say "give me the latest 2.x and at minimum 2.2"? This is more like what I'd expect from a version specifier.

@laupow
Copy link
Collaborator Author

laupow commented Apr 5, 2023

@wraithan The PR now adds support for semver device-os-version inputs.

How do the usage docs look for your use-case? Tilde versions are supported but not documented.

compile-action/README.md

Lines 20 to 28 in 0f36975

# Target Device OS firmware version
# Allowed values:
# latest: the most recent Device OS version for the platform
# latest-lts: the most recent LTS Device OS version for the platform
# <version>: a specific Device OS version, e.g. 2.3.1
# ^<version>: a semver range, e.g. ^5.3.0
# For production projects, you should pin to a specific Device OS semver range, e.g. ^4.0.0
# Required: false
device-os-version: 'latest-lts'

@laupow laupow marked this pull request as ready for review April 5, 2023 10:26
@laupow laupow requested review from wraithan and hugomontero April 5, 2023 12:08
@laupow laupow changed the title feat: [sc-117417] Add device-os-output variable feat: [sc-117417] Add semver support and device-os-output variable Apr 5, 2023
@wraithan
Copy link

wraithan commented Apr 5, 2023

@wraithan The PR now adds support for semver device-os-version inputs.

How do the usage docs look for your use-case? Tilde versions are supported but not documented.

Carrot ranges are good too, they're default in npm for a reason. Works for me, looks like the build fails when latest-lts can't find an LTS version, that works for me. Wonder if we should document that or if it would be too verbose/discourage use.

The note about production is also interesting, our users do not use our platform that way, they definitely hardcode a version and leave it forever in most cases, which they can do by omiting the carrot so it's fine. Upgrades within a major, especially if the device has kept up on OS versions will (on 2.x+) be combined OTA as well as having incremental. So in many cases can skip safe mode so it feels like it'd be fine to suggest using a semver range in production, but the more risk adverse or those burned by safe mode healer + bad connectivity in the past will almost certainly hardcode an OS version.

@wraithan
Copy link

wraithan commented Apr 5, 2023

I appreciate how well tested all this code is, I was able to scroll through the tests and see how things would react, very nice.

@laupow laupow merged commit f743514 into main Apr 6, 2023
@laupow laupow deleted the feature/sc-117417/output-variable branch April 6, 2023 11:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants