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

Added str_word_count to the stub file (fix #6016) #6069

Merged
merged 2 commits into from
Jul 9, 2021

Conversation

elnoro
Copy link
Contributor

@elnoro elnoro commented Jul 9, 2021

Hello

If I need to add any tests, please tell me where.
Thank you!

stubs/CoreGenericFunctions.phpstub Outdated Show resolved Hide resolved
@weirdan
Copy link
Collaborator

weirdan commented Jul 9, 2021

We generally don't have tests for most of the stubs.

Co-authored-by: Bruce Weirdan <weirdan@gmail.com>
@elnoro
Copy link
Contributor Author

elnoro commented Jul 9, 2021

Ok, so it failed on one of the test projects
The issue reported by Psalm is UnnecessaryVarAnnotation https://github.com/muglug/psl/blob/0.1.x/src/Psl/Str/Byte/words.php#L22.
Psalm could not figure out the type of the variable before, but it now can and therefore the annotation became obsolete. Should I send a PR to the library then?

@caugner
Copy link
Contributor

caugner commented Jul 9, 2021

Should I send a PR to the library then?

That's probably a good idea, but make sure to create the PR in muglug's fork, which is used in the "test-with-real-projects" (see below).

@weirdan
Copy link
Collaborator

weirdan commented Jul 9, 2021

That's probably a good idea, but make sure to create the PR in muglug's fork, which is used in the "test-with-real-projects".

It's better to send the PR upstream. For muglug's fork we would just baseline the issue.

@weirdan
Copy link
Collaborator

weirdan commented Jul 9, 2021

Should I send a PR to the library then?

Feel free to do that, but it's not a prerequisite to get this merged.

@weirdan weirdan merged commit f381fba into vimeo:master Jul 9, 2021
@caugner
Copy link
Contributor

caugner commented Jul 9, 2021

It's better to send the PR upstream. For muglug's fork we would just baseline the issue.

Oh, I just saw earlier that the fork's 1.6.x branch is tested, while upstream is already on 1.7.x, so I assumed the fork is not kept in sync with upstream anymore.

@elnoro elnoro deleted the str-word-count-stub branch July 13, 2021 10:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants