Movatterモバイル変換

[0]ホーム
URL:

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

[AssetMapper] ignore missing directory inisVendor()#58859

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation

alexislefebvre
Copy link
Contributor

@alexislefebvrealexislefebvre commentedNov 13, 2024•
edited
Loading

QA
Branch?6.4
Bug fix?yes
New feature?no
Deprecations?no
IssuesFix#58858
LicenseMIT

I don't know how to add tests yet, because this method is private.

@carsonbotcarsonbot added this to the7.2 milestoneNov 13, 2024
@alexislefebvrealexislefebvreforce-pushed the6.4-ignore-missing-directory-in-isVendor branch from40ac31b tod38fc98CompareNovember 13, 2024 17:38
@alexislefebvrealexislefebvre changed the base branch from7.2 to6.4November 13, 2024 17:39
@alexislefebvrealexislefebvre changed the titlefix: ignore missing directory in isVendor()fix: ignore missing directory inisVendor()Nov 13, 2024
@alexislefebvrealexislefebvre changed the titlefix: ignore missing directory inisVendor()[AssetMapper] ignore missing directory inisVendor()Nov 13, 2024
@alexislefebvrealexislefebvreforce-pushed the6.4-ignore-missing-directory-in-isVendor branch 2 times, most recently from74495f5 to88ab9b0CompareNovember 14, 2024 16:26
@OskarStarkOskarStark modified the milestones:7.2,6.4Nov 14, 2024
@smnandre
Copy link
Member

Arf sorry.... i did not see it :(

#58880

@carsonbotcarsonbot changed the title[AssetMapper] ignore missing directory inisVendor()ignore missing directory inisVendor()Nov 14, 2024
@alexislefebvre
Copy link
ContributorAuthor

alexislefebvre commentedNov 15, 2024•
edited
Loading

Arf sorry.... i did not see it :(

#58880

Your PR relies on a simple mock, while my PR changes$this->createFactory(…);. Your test looks cleaner, should we use it in this current PR instead of changingcreateFactory()?

@carsonbotcarsonbot changed the titleignore missing directory inisVendor()[AssetMapper] ignore missing directory inisVendor()Nov 15, 2024
@smnandre
Copy link
Member

Arf sorry.... i did not see it :(
#58880

Your PR relies on a simple mock, while my PR changes$this->createFactory(…);. Your test looks cleaner, should we use it in this current PR instead of changingcreateFactory()?

No idea 🤷

But if you want to take anything from my closed PR you can of course :)

@alexislefebvre
Copy link
ContributorAuthor

No idea 🤷

But if you want to take anything from my closed PR you can of course :)

Thanks for the feedback, let's see the opinions of the reviewers. 🙂

@symfonysymfony deleted a comment fromcarsonbotNov 20, 2024
@nicolas-grekasnicolas-grekasforce-pushed the6.4-ignore-missing-directory-in-isVendor branch from88ab9b0 to9e3984fCompareNovember 20, 2024 11:01
@nicolas-grekas
Copy link
Member

Thank you@alexislefebvre.

alexislefebvre reacted with rocket emoji

@nicolas-grekasnicolas-grekas merged commit10be4d6 intosymfony:6.4Nov 20, 2024
7 of 9 checks passed
@nicolas-grekas
Copy link
Member

Less mocks FTW

alexislefebvre reacted with laugh emoji

@alexislefebvrealexislefebvre deleted the 6.4-ignore-missing-directory-in-isVendor branchNovember 20, 2024 13:00
@fabpotfabpot mentioned this pull requestNov 27, 2024
This was referencedNov 27, 2024
nicolas-grekas added a commit that referenced this pull requestJan 7, 2025
…lexislefebvre)This PR was merged into the 7.3 branch.Discussion----------[AssetMapper] use constants in MappedAssetFactoryTest| Q             | A| ------------- | ---| Branch?       | 7.3| Bug fix?      | no| New feature?  | no| Deprecations? | no| Issues        | no| License       | MITThis avoids repeating `__DIR__` in several places.Follow-up of:-#58859Commits-------cdb3080 feat: use constants in MappedAssetFactoryTest
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@chalasrchalasrchalasr approved these changes

Assignees
No one assigned
Projects
None yet
Milestone
6.4
Development

Successfully merging this pull request may close these issues.

[AssetMapper]MappedAssetFactory->isVendor() returntrue when the directory is missing
7 participants
@alexislefebvre@smnandre@nicolas-grekas@chalasr@OskarStark@xabbuh@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp