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

Fix for Issue 61081 #61221

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Fix for Issue 61081 #61221

wants to merge 6 commits into from

Conversation

JulianTaub
Copy link

@JulianTaub JulianTaub commented Feb 19, 2025

Fixes #61081

@typescript-bot typescript-bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Feb 19, 2025
@typescript-bot
Copy link
Collaborator

This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise.

@typescript-bot typescript-bot added For Backlog Bug PRs that fix a backlog bug and removed For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Feb 19, 2025
@JulianTaub
Copy link
Author

@JulianTaub please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.

@microsoft-github-policy-service agree [company="{your company}"]

Options:

  • (default - no company specified) I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer.
@microsoft-github-policy-service agree
  • (when company given) I am making Submissions in the course of work for my employer (or my employer has intellectual property rights in my Submissions by contract or applicable law). I have permission from my employer to make Submissions and enter into this Agreement on behalf of my employer. By signing below, the defined term “You” includes me and my employer.
@microsoft-github-policy-service agree company="Microsoft"

Contributor License Agreement

@microsoft-github-policy-service agree

@JulianTaub
Copy link
Author

@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.

@jakebailey
Copy link
Member

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.

@JulianTaub
Copy link
Author

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.
Copy link

Choose a reason for hiding this comment

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

possible typo?, "coverted"

Copy link
Author

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
Copy link

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.

Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Backlog Bug PRs that fix a backlog bug
Projects
Status: Not started
Development

Successfully merging this pull request may close these issues.

Array.splice "deleteCount" argument description is somewhat insufficient
4 participants