Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Resolve remaining predefined constants #693

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

Merged
ondrejmirtes merged 2 commits into phpstan:master from herndlm:feature/resolve-remaining-predefined-constants
Jan 30, 2022
Merged

Resolve remaining predefined constants #693

ondrejmirtes merged 2 commits into phpstan:master from herndlm:feature/resolve-remaining-predefined-constants
Jan 30, 2022

Conversation

@herndlm
Copy link
Contributor

@herndlm herndlm commented Sep 29, 2021
edited
Loading

Follow-up of #684

This resolves the remaining predefined constants that are configured via dynamicConstantNames and differ from what reflection is able to resolve.

There are a few constants that are defined via compilation flags where I was not a 100% sure if they can be empty or not.

And, of course, there would be many more constants from extensions that could be defined potentially and the resolving code could be refactored or grouped differently, but for now I wanted to keep it simple and adapt it based on the feedback I get.
Currently it's grouped/sorted like the following with links to docs

  • group of core constants
  • group of "other" core constants
  • constants grouped by extensions which are sorted alphabetically
  • constants inside groups are sorted as they occur in the linked docs

canvural reacted with thumbs up emoji voku and mvorisek reacted with heart emoji
Copy link
Contributor Author

herndlm commented Oct 13, 2021

What do you think @ondrejmirtes and what about the remaining open conversations @staabm?

Copy link
Contributor Author

herndlm commented Jan 2, 2022

rebased and cleaned up a bit

Copy link
Member

Hi, I really don’t want to read 64 comments. Anything worth my attention before I review? 😊

Copy link
Contributor Author

herndlm commented Jan 13, 2022
edited
Loading

Sorry for spaming you with the rebases (wanted to make the CI green again) :/

Most of it was about the PHP version constants (should we have upper limits or not). So a look/check/special think there might be worth when reviewing. The rest is resolved.

This resolves the remaining predefined constants that are configured via `dynamicConstantNames` and differ from what reflection is able to resolve.
@ondrejmirtes ondrejmirtes merged commit da80c29 into phpstan:master Jan 30, 2022
Copy link
Member

Thank you!

@herndlm herndlm deleted the feature/resolve-remaining-predefined-constants branch January 30, 2022 13:27
Copy link
Contributor Author

herndlm commented Jan 30, 2022

thx, I hope this doesn't introduce annoying new errors 🤞

Copy link
Member

I’ll /cc you if it does 😊

herndlm reacted with laugh emoji

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ondrejmirtes ondrejmirtes ondrejmirtes left review comments

+4 more reviewers

@staabm staabm staabm left review comments

@mvorisek mvorisek mvorisek left review comments

@orklah orklah orklah left review comments

@dktapps dktapps dktapps left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /