Skip to content

Conversation

@Log1x
Copy link
Member

@Log1x Log1x commented Apr 14, 2020

No description provided.

@coveralls
Copy link

coveralls commented Apr 14, 2020

Pull Request Test Coverage Report for Build 51

  • 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 100.0%

Totals Coverage Status
Change from base Build 47: 0.0%
Covered Lines: 19
Relevant Lines: 19

💛 - Coveralls

Copy link
Collaborator

@austinpray austinpray left a comment

Choose a reason for hiding this comment

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

added some failing test cases 5244479

Co-Authored-By: Austin Pray <[email protected]>
@retlehs retlehs requested a review from QWp6t January 26, 2024 16:36
@tangrufus
Copy link
Member

tangrufus commented Jan 30, 2025

Discussion: If Config::get() returns default values, do we still need UndefinedConfigKeyException?

In my opinion, since we always have a default value ($default = null), we never need to throw UndefinedConfigKeyException.
If users really need to check whether a key is defined in the configMap, we can add a Config::has() method.

I vote for something similar to https://github.com/laravel/framework/blob/4a74d20f8d960b38d75dd525946e893c75a94dbc/src/Illuminate/Config/Repository.php#L33-L58


But, see

I throw a `UndefinedConfigKeyException` when a user attempts to fetch a value with `Config::get(k)` that doesn't exist in `Config::$configMap`. I do this because the control flow is nice and `return null` is not working because null is a viable configuration value. We don't use `Config::get(k)` anywhere but it is a part of the API because I can see users needing it.

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.

5 participants