-
Notifications
You must be signed in to change notification settings - Fork 39
[MOB-12231] refactor-classes-to-reduce-circular-dependencies #731
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
Conversation
…SDK initialization and logging
…ions and usage examples
6 new issues
This is from Qlty Cloud, the successor to Code Climate Quality. Learn more. |
…lure' into MOB-12231-refactor-classes-to-reduce-circular-dependencies
…lure' into MOB-12231-refactor-classes-to-reduce-circular-dependencies
…rectly in relevant classes
Diff Coverage: The code coverage on the diff in this pull request is 82.4%. Total Coverage: This PR will increase coverage by 3.98%. File Coverage Changes
🛟 Help
This is from Qlty Cloud, the successor to Code Climate Quality. Learn more. |
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.
Partially reviewed
"prepare": "yarn build", | ||
"release": "release-it" | ||
"release": "release-it", | ||
"circ": "npx madge --circular --extensions ts ./" |
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.
Is it a dedicated package to results circular dependency?
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.
yup!
src/core/classes/Iterable.ts
Outdated
import type { IterableCommerceItem } from './IterableCommerceItem'; | ||
import { IterableConfig } from './IterableConfig'; | ||
import { IterableLogger } from './IterableLogger'; | ||
import type { IterableAuthFailure } from '../types/IterableAuthFailure'; |
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.
Import 'type' is removed above. Is this one ok?
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.
Yes. The issue was coming from the use of Iterable.logger
in IterableInAppManager
. This has been changed to not require that anymore.
src/core/classes/Iterable.ts
Outdated
|
||
const RNEventEmitter = new NativeEventEmitter(RNIterableAPI); | ||
|
||
const defaultConfig = new IterableConfig(); |
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.
Not sure how default implementation comes into play. Any possibility of regression?
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.
No -- it's the same functionality as was there before -- it's just assigning it to a variable
…gging behavior and hierarchy
…sses' into jwt/MOB-10947-task-3-android-retrypolicy-config-at-android-br
…oid-br' into MOB-12231-refactor-classes-to-reduce-circular-dependencies
…thManager from Iterable class
static initialize2WithApiKey( | ||
apiKey: string, | ||
config: IterableConfig = new IterableConfig(), | ||
version: string, | ||
apiEndPoint: string |
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.
static trackPushOpenWithCampaignId( | ||
campaignId: number, | ||
templateId: number, | ||
messageId: string | null | undefined, | ||
appAlreadyRunning: boolean, | ||
dataFields?: unknown |
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.
static trackInAppClose( | ||
message: IterableInAppMessage, | ||
location: IterableInAppLocation, | ||
source: IterableInAppCloseSource, | ||
clickedUrl?: string |
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.
static updateSubscriptions( | ||
emailListIds: number[] | null, | ||
unsubscribedChannelIds: number[] | null, | ||
unsubscribedMessageTypeIds: number[] | null, | ||
subscribedMessageTypeIds: number[] | null, | ||
campaignId: number, | ||
templateId: number |
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.
🔹 JIRA Ticket(s) if any
✏️ Description