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

Add int_range type #510

Open
wants to merge 1 commit into
base: next
Choose a base branch
from
Open

Add int_range type #510

wants to merge 1 commit into from

Conversation

gsteel
Copy link
Contributor

@gsteel gsteel commented Feb 26, 2025

Ref #390

Unfortunately, Psalm does not appear to support templates in int ranges, i.e. int<TMin, TMax>

What's the best thing to do here? reduce the templates to int and do something in the psalm plugin?

@coveralls
Copy link

coveralls commented Feb 26, 2025

Pull Request Test Coverage Report for Build 13628044181

Details

  • 31 of 31 (100.0%) changed or added relevant lines in 2 files are covered.
  • 2 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-0.03%) to 98.472%

Files with Coverage Reduction New Missed Lines %
src/Psl/DateTime/Internal/high_resolution_time.php 2 80.95%
Totals Coverage Status
Change from base Build 13624899182: -0.03%
Covered Lines: 5478
Relevant Lines: 5563

💛 - Coveralls

@gsteel
Copy link
Contributor Author

gsteel commented Feb 27, 2025

Added a rather hopeful feature request here too: vimeo/psalm#11343

@azjezz
Copy link
Owner

azjezz commented Mar 3, 2025

Could you please update the annotations to make psalm happy? either way psalm won't understand it. we can add proper templates later once psalm adds support for this.

rebase is needed as well.

@azjezz azjezz added Priority: Medium This issue may be useful, and needs some attention. Status: Revision Needed At least two people have seen issues in the PR that makes them uneasy. Type: Enhancement Most issues will probably ask for additions or changes. labels Mar 3, 2025
@gsteel
Copy link
Contributor Author

gsteel commented Mar 3, 2025

@azjezz Looks like the windows test failure is un-related

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: Medium This issue may be useful, and needs some attention. Status: Revision Needed At least two people have seen issues in the PR that makes them uneasy. Type: Enhancement Most issues will probably ask for additions or changes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants