-
Notifications
You must be signed in to change notification settings - Fork 27
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
add set version script #407
Conversation
pod lib lint --allow-warnings RadarSDKMotion.podspec | ||
pod trunk push --allow-warnings RadarSDK.podspec | ||
pod trunk push --allow-warnings RadarSDKMotion.podspec |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we definitely want to lint both before publishing any of the two.
@@ -949,7 +949,6 @@ | |||
"@executable_path/Frameworks", | |||
"@loader_path/Frameworks", | |||
); | |||
MARKETING_VERSION = 3.9.6; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is the package settings, which overwrites the project settings, we should just have the version set for project settings which will be resolved everywhere to the single correct marketing version
@@ -979,7 +978,6 @@ | |||
"@executable_path/Frameworks", | |||
"@loader_path/Frameworks", | |||
); | |||
MARKETING_VERSION = 3.9.6; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as above
@@ -347,6 +348,7 @@ | |||
GCC_WARN_UNUSED_VARIABLE = YES; | |||
IPHONEOS_DEPLOYMENT_TARGET = 12.0; | |||
LOCALIZATION_PREFERS_STRING_CATALOGS = YES; | |||
MARKETING_VERSION = 3.18.4; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
adding the marketing version to motion sdks to be the same as main sdk. (project setting)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm gonna test this with a bump today to verify it's good before 👍, but this looks good. Thank you — nice proactive change.
set_version.sh
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there's also a CFBundleShortVersionString
in the Info.plist
that has lagged for a while and we don't generally update. We should get that too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CFBundleShortVersionString is set to parameter substitution of #(MARKETING_VERSION) so we are good there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
script works well from my testing and the changes to release script makes sense imo
* add set version script * improve CI steps to detect errors earlier --------- Co-authored-by: KennyHuRadar <139801512+KennyHuRadar@users.noreply.github.com>
There are two components of this PR, improved CI checks, and version updating script.
This should make sure that if CI passes, both are successfully uploaded, and if CI fails, neither are uploaded.
(There was no documentation on bumping MotionSDK in notion, which should have been there, I've updated the notion document)