fix(copier): broken log message #39230
Conversation
|
I can't help but feel icky that we can change this code back and forth without breaking any test. That also makes it hard for me to grasp why out of the 8 cases in #38897 seemingly matching the pattern, only 6 need a fix. |
Agreed. I will add tests for these cases as well. Regarding 6/8 cases needing the change. The other 2 cases are |
|
Can we add misisng tests? Then we can check that things are broken before this PR's changes, then when applying this change, they should be fixed? |
| config, | ||
| }: UpdateArtifact): Promise<UpdateArtifactsResult[] | null> { | ||
| if (updatedDeps?.length !== 1) { | ||
| if (isNullOrUndefined(updatedDeps) || updatedDeps.length !== 1) { |
There was a problem hiding this comment.
| if (isNullOrUndefined(updatedDeps) || updatedDeps.length !== 1) { | |
| if (updatedDeps?.length !== 1) { |
This is fine as the logic behaves correctly when updateDeps is null | undefined. But, the debug message inside the if-check breaks, so need to fix that.
There was a problem hiding this comment.
agreed, use the optional chaining here
There was a problem hiding this comment.
This can be reverted. If the sha256=null|undefined the logic is working fine.
There's also a test to confirm this already present.
There was a problem hiding this comment.
The existing expression is not broken as the value will be compared to a constant value.
There was a problem hiding this comment.
so this change isn't required? please leave as is then
|
@astellingwerf Could you please review again. I have added tests where possible and have added a comment on the other file where the existing logic is not breaking or a test cannot be added. |
astellingwerf
left a comment
There was a problem hiding this comment.
Thanks, @RahulGautamSingh!
Changes
isNullOrUndefinedcheck instead of?(optional chaining) which can let some edge cases inContext
Please select one of the below:
AI assistance disclosure
Did you use AI tools to create any part of this pull request?
Please select one option and, if yes, briefly describe how AI was used (e.g., code, tests, docs) and which tool(s) you used.
Documentation (please check one with an [x])
How I've tested my work (please select one)
I have verified these changes via: