Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Use enum instead of interface for state #5372

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Use enum instead of interface for state #5372

wants to merge 3 commits into from
+60 −104

Conversation

docjyJ
Copy link
Collaborator

@docjyJ docjyJ commented Oct 2, 2024

I use VersionState instade of bool

@docjyJ docjyJ added 3. to review Waiting for reviews enhancement New feature or request labels Oct 2, 2024
@docjyJ docjyJ added this to the next milestone Oct 2, 2024
@docjyJ docjyJ requested a review from st3iny October 2, 2024 13:18
@docjyJ docjyJ self-assigned this Oct 2, 2024
This was referenced Oct 2, 2024
Draft
Open
@szaimen szaimen mentioned this pull request Oct 4, 2024
Open
21 tasks
st3iny
st3iny approved these changes Oct 6, 2024
View reviewed changes
Copy link
Member

@st3iny st3iny left a comment

Choose a reason for hiding this comment

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

Looks good but did not test.

@szaimen I'd say we are good to go if you are fine with the naming:
IContainerState was refactored to enums WorkingState and VersionState

@docjyJ
Copy link
Collaborator Author

docjyJ commented Oct 6, 2024

I didn't test to. Workflow pass and @szaimen you have better test instance than me 😅

@docjyJ
Copy link
Collaborator Author

docjyJ commented Oct 6, 2024

IContainerState was refactored to enums WorkingState and VersionState

I split them in two, because it didn't make sense for the version and work status to be the same interface...

szaimen
szaimen requested changes Oct 7, 2024
View reviewed changes
php/src/Container/WorkingState.php Outdated Show resolved Hide resolved
@szaimen szaimen changed the title Use enum instade of interface for state Use enum instead of interface for state Oct 7, 2024
docjyJ and others added 3 commits October 7, 2024 13:22
@docjyJ
Add Enum
a0ec043 
Signed-off-by: Jean-Yves <[email protected]>
@docjyJ
Fix containers.twig
edeb5ca 
Signed-off-by: Jean-Yves <[email protected]>
@docjyJ @szaimen
Update php/src/Container/WorkingState.php
dc209ad 
Co-authored-by: Simon L. <[email protected]>
Signed-off-by: Jean-Yves <[email protected]>
@docjyJ docjyJ requested a review from szaimen October 7, 2024 11:24
@docjyJ
Copy link
Collaborator Author

docjyJ commented Oct 7, 2024 •
edited
Loading

Done, #5307 will be ready to

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

@st3iny st3iny st3iny approved these changes

@szaimen szaimen Awaiting requested review from szaimen

Requested changes must be addressed to merge this pull request.

Assignees

@docjyJ docjyJ

Labels
3. to review Waiting for reviews enhancement New feature or request
Projects
None yet
Milestone
next
Development

Successfully merging this pull request may close these issues.
3 participants
@docjyJ @st3iny @szaimen