-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Fix for Issue 61081 #61221
base: main
Are you sure you want to change the base?
Fix for Issue 61081 #61221
Conversation
This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise. |
@microsoft-github-policy-service agree |
@typescript-bot I saw that CI/ format check failed. I just formatted the file locally and I can push it up and add it to the pull request after the request has been reviewed. |
FWIW you do not need tests for this kind of change; it's honestly worse to write tests for these comments because updating them later will mean having to update tests to just copy the same text a second time. |
Thanks @jakebailey, I understand. I can take out my 2 tests if necessary. Also, I just pushed up 3 baseline/reference test files that had failed after reformatting. |
src/lib/es5.d.ts
Outdated
* @param deleteCount The number of elements to remove. | ||
* @param deleteCount The number of elements to remove. Omitting this argument will remove all elements from the start | ||
* paramater location to end of the array. If value of this argument is either a negative number, zero, undefined, or a type | ||
* that cannot be coverted to an integer, the function will evaluate the argument as zero and not remove any elements. |
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.
possible typo?, "coverted"
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.
Hi @birgersp, yes it is a typo. I just fixed it and pushed it up.
src/lib/es5.d.ts
Outdated
* @param deleteCount The number of elements to remove. If value of this argument is either a negative number, zero, | ||
* undefined, or a type that cannot be coverted to an integer, the function will evaluate the argument as zero and | ||
* not remove any elements. | ||
* Note: If the deleteCount argument is left empty between the start and items arguments, it will not be evaluated as |
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.
what does "left empty" mean, specifically?
if you mean "omitted" then that's not really valid, because the compiler will throw an error; "No overload matches this call". IMO this note can be removed.
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.
@birgersp, yes I meant omitted. I just took out that sentence (that started with "Note") and pushed it up in the latest commit.
Fixes #61081