-
Notifications
You must be signed in to change notification settings - Fork 2
feat: Reclassify diffs to risky in a merged document for no-bwc operation #54
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
base: feature/performance-optimization-405
Are you sure you want to change the base?
feat: Reclassify diffs to risky in a merged document for no-bwc operation #54
Conversation
| expect.objectContaining({ | ||
| scope: 'response', | ||
| action: DiffAction.replace, | ||
| type: risky, |
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.
extend expected values with "before/afterDeclarationPaths" to clarify where diffs are expected.
you may use expect.arrayContaining matcher to avoid copy-pasting whole addresses, if you want.
| expect.objectContaining({ | ||
| scope: 'response', | ||
| action: DiffAction.replace, | ||
| type: risky, |
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.
extend expected values with "before/afterDeclarationPaths" to clarify where diffs are expected.
you may use expect.arrayContaining matcher to avoid copy-pasting whole addresses, if you want.
| expect.objectContaining({ | ||
| scope: 'response', | ||
| action: DiffAction.replace, | ||
| type: breaking, |
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.
extend expected values with "before/afterDeclarationPaths" to clarify where diffs are expected.
you may use expect.arrayContaining matcher to avoid copy-pasting whole addresses, if you want.
| expect.objectContaining({ | ||
| scope: 'components', | ||
| action: DiffAction.replace, | ||
| type: breaking, |
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.
extend expected values with "before/afterDeclarationPaths" to clarify where diffs are expected.
you may use expect.arrayContaining matcher to avoid copy-pasting whole addresses, if you want.
| ])) | ||
| }) | ||
|
|
||
| it('should mark GET method as risky and POST as breaking when only GET is in scope', async () => { |
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.
One of these tests is excessive, since they are testing the same case, just on different data. One is enough, remove another one.
'should mark GET method as risky and POST as breaking when only GET is in scope'
'should mark POST method as breaking and GET as risky when only POST is in scope'
| import { | ||
| AdapterContext, | ||
| AdapterResolver, | ||
| BwcState, |
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.
Let's be consistent with naming.
BwcState -> ApiCompatibilityScope
ApiCompatibilityKind: OK
backwardCompatibility -> apiCompatibilityScope
bwcScopeFunction -> apiCompatibilityScopeFunction
BwcScopeFunction -> ApiCompatibilityScopeFunction
parentBwc -> parentApiCompatibilityScope
computedBwc -> computedApiCompatibilityScope
| "value": { | ||
| "type": "string" | ||
| }, | ||
| "currency": { |
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 need to have two fields here, simplify the case by removing field which is not changed.
| onCreateDiffError?: (message: string, diff: Diff, ctx: CompareContext) => void | ||
| beforeValueNormalizedProperty?: symbol | ||
| afterValueNormalizedProperty?: symbol | ||
| bwcScopeFunction?: BwcScopeFunction |
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.
Add a documentation for this function to clarify it's intended use-case.
… operation