Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add Tag attribute #52743

Merged
merged 1 commit into from
Sep 11, 2024
Merged

Add Tag attribute #52743

merged 1 commit into from
Sep 11, 2024

Conversation

TijmenWierenga
Copy link
Contributor

This PR makes it easier to inject tags into services.
Instead of manually having to bind the instance that uses the tag, you can now reference the tag in the target class.

Before:

$this->app->bind(ReportAnalyzer::class, function (Application $app) {
    return new ReportAnalyzer($app->tagged('reports'));
});

After:

class ReportAnalyzer
{
    public function __construct(
        #[Tag('reports')] iterable $reports
    )
}

@taylorotwell taylorotwell merged commit 77739f6 into laravel:11.x Sep 11, 2024
31 checks passed
@johanrosenson
Copy link
Contributor

I see this has already been merged, but shouldn't the attribute be named Tagged to better match the tagged() method?

@TijmenWierenga
Copy link
Contributor Author

I see this has already been merged, but shouldn't the attribute be named Tagged to better match the tagged() method?

I considered both Tag and Tagged. In the end, I went for Tag because it's shorter and reads a little nicer. You want to 'inject a tag named {x}'. Does that make sense to you? 🙂

@johanrosenson
Copy link
Contributor

I'm reading it as "inject everything tagged {x}", because you are not really injecting the tag, you are injecting everything tagged.

Tag for me is an action, "tag that with x"

@TijmenWierenga
Copy link
Contributor Author

I'm reading it as "inject everything tagged {x}", because you are not really injecting the tag, you are injecting everything tagged.

Tag for me is an action, "tag that with x"

I can follow your logic as well. I honestly wouldn't care if someone decides to change it. Same goes for keeping it like that 😅 .

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants