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

update the BT min Version, add patchVersion + update unit test #2435

Merged
merged 3 commits into from
Oct 3, 2024

Conversation

hnguyen510
Copy link
Contributor

@hnguyen510 hnguyen510 commented Sep 20, 2024

Description

Create a Default Version BT version to 3.107.1

@hnguyen510 hnguyen510 requested a review from a team as a code owner September 20, 2024 20:35
@hnguyen510 hnguyen510 marked this pull request as draft September 23, 2024 14:07
@hnguyen510 hnguyen510 marked this pull request as ready for review September 23, 2024 19:41
}
const versionSplit = version.split(".");
// patch could have an alpha tag
const patchSplit = versionSplit[2].split("-");
Copy link
Member

Choose a reason for hiding this comment

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

do we need to validate the patch version? Is the fastlane team asking for that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Me and Nik thought it something good to have but we can remove it.

Copy link
Contributor

Choose a reason for hiding this comment

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

With the DEFAULT_BT_VERSION & MIN_BT_VERSION having patch versions greater that 0, it seemed like a valid check.

Copy link
Member

Choose a reason for hiding this comment

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

the only thing i'm worried about is accidentally marking current integrations as invalid. Technically, if this check is working correctly, then the merchant's integration wouldn't work anyway.

but i would check with the fastlane team to see if they have any dashboards we can watch while the SDK deploys

Copy link
Contributor

Choose a reason for hiding this comment

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

With the current logic anything below 3.97.3 will throw an error, the previous logic would only throw an error for versions less than 3.97

It seems like the min version is on this specific version 3.97.3-connect-alpha.6.1
If a merchant was on 3.97.1, no error would be thrown, but I don't think that version would have the Fastlane changes?

Is that logic correct?

Copy link
Contributor

Choose a reason for hiding this comment

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

i don't think this will break anything, but i also don't think it's necessary.
just for context, the first official version of BT to support Fastlane is 3.104.0. before that, the only other versions with Fastlane were:
3.95.0-connect-alpha.12
3.97.3-connect-alpha.6.1
3.103.0-fastlane-beta.7.3

@nikrom17 nikrom17 force-pushed the feature/update-min-BT-version branch from ea7fcf5 to 1c381eb Compare October 2, 2024 17:06
@hnguyen510 hnguyen510 merged commit 05f95db into main Oct 3, 2024
3 checks passed
@hnguyen510 hnguyen510 deleted the feature/update-min-BT-version branch October 3, 2024 15:09
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.

4 participants