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

[Tests] Streamline#52402

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

Merged

Conversation

@OskarStark
Copy link
Contributor

@OskarStarkOskarStark commentedNov 1, 2023•
edited
Loading

@OskarStarkOskarStark self-assigned thisNov 1, 2023
@carsonbotcarsonbot added this to the6.4 milestoneNov 1, 2023

(newStateMachineValidator())->validate($definition,'foo');

// the test ensures that the validation does not fail (i.e. it does not throw any exceptions)
Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

are the following lines still needed@lyrixx I don't understand that part 🤔

Choose a reason for hiding this comment

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

this makes no sense I agree, let's see what happens when removing the next lines

Choose a reason for hiding this comment

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

could be also the result of a bad merge ? how does this look like on lower branches?

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

The code looks the same until 4.4, I will provide a PR against 5.4 for this test and revert here

@OskarStarkOskarStark requested a review fromstofNovember 1, 2023 08:34
@OskarStarkOskarStarkforce-pushed thefeature/streamline-tests-6.4 branch from7add3d7 tobac4e6cCompareNovember 1, 2023 13:23
nicolas-grekas added a commit that referenced this pull requestNov 2, 2023
…tional arguments in tests (OskarStark)This PR was merged into the 5.4 branch.Discussion----------[PasswordHasher] [Tests] Do not invoke methods with additional arguments in tests| Q             | A| ------------- | ---| Branch?       | 5.4| Bug fix?      | no| New feature?  | no| Deprecations? | no| Issues        | ---| License       | MITExtracted from#52402I think its worth to merge this in 5.4Commits-------fdaefc4 [PasswordHasher][Tests] Do not invoke methods with additional arguments in tests
@OskarStarkOskarStarkforce-pushed thefeature/streamline-tests-6.4 branch from27946e7 to10b0f48CompareDecember 20, 2023 23:11
@OskarStark
Copy link
ContributorAuthor

How easy would it be for you to revert all the added return types from providers?
They don't provide any benefit IMHO but make reviewing harder + will generate merge needless conflicts.

Done, good to go@nicolas-grekas

@OskarStarkOskarStark mentioned this pull requestDec 20, 2023
fabpot added a commit that referenced this pull requestDec 21, 2023
This PR was merged into the 5.4 branch.Discussion----------[Workflow] Fix test| Q             | A| ------------- | ---| Branch?       | 5.4| Bug fix?      | no| New feature?  | no| Deprecations? | no| Issues        | Follows#52402 (comment)| License       | MITCommits-------02fa2ec [Workflow] Fix test
@OskarStarkOskarStarkforce-pushed thefeature/streamline-tests-6.4 branch from10b0f48 toee89a98CompareDecember 21, 2023 11:22
@xabbuh
Copy link
Member

the failures look related

@OskarStark
Copy link
ContributorAuthor

the failures look related

fixed

@OskarStark
Copy link
ContributorAuthor

Rebased ✅

@nicolas-grekasnicolas-grekasforce-pushed thefeature/streamline-tests-6.4 branch from9d64921 to33d71aaCompareDecember 26, 2023 14:02
@nicolas-grekas
Copy link
Member

Thank you@OskarStark.

OskarStark reacted with heart emoji

@nicolas-grekasnicolas-grekas merged commite6acb8c intosymfony:7.1Dec 26, 2023
fabpot added a commit that referenced this pull requestDec 29, 2023
…ark)This PR was merged into the 5.4 branch.Discussion----------[Tests] Streamline `CompiledUrlGenerator` tests| Q             | A| ------------- | ---| Branch?       | 7.1| Bug fix?      | no| New feature?  | no| Deprecations? | no| Issues        | Follows#52402| License       | MITweird, it looks like the expected exception is thrown in $this->generatorDumper->dump() 🤔 So the following code with the CompiledUrlGenerator is not needed, so maybe the test is not needed anymore or the test must be adjusted somehow to test the behavior of CompiledUrlgenerator....Commits-------807d70e [Tests] Streamline CompiledUrlGenerator tests
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@nicolas-grekasnicolas-grekasnicolas-grekas left review comments

@derrabusderrabusderrabus left review comments

@xabbuhxabbuhxabbuh approved these changes

@lyrixxlyrixxAwaiting requested review from lyrixxlyrixx is a code owner

@stofstofAwaiting requested review from stof

@welcoMatticwelcoMatticAwaiting requested review from welcoMatticwelcoMattic is a code owner

@dunglasdunglasAwaiting requested review from dunglasdunglas is a code owner

@chalasrchalasrAwaiting requested review from chalasrchalasr is a code owner

Assignees

@OskarStarkOskarStark

Projects

None yet

Milestone

7.1

Development

Successfully merging this pull request may close these issues.

6 participants

@OskarStark@nicolas-grekas@xabbuh@derrabus@chalasr@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp