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

Split symfony/symfony #6817

Merged
merged 1 commit into from Aug 11, 2023
Merged

Split symfony/symfony #6817

merged 1 commit into from Aug 11, 2023

Conversation

yguedidi
Copy link
Contributor

@yguedidi yguedidi commented Aug 8, 2023

I hope I missed nothing, at least tests pass

@Kdecherf
Copy link
Member

Kdecherf commented Aug 9, 2023

Should we target milestone 2.7 for this PR? @wallabag/core @yguedidi

@yguedidi
Copy link
Contributor Author

yguedidi commented Aug 9, 2023

To me it's a dependency refactoring, so it could fit in 2.6, but up to you :)

@yguedidi
Copy link
Contributor Author

yguedidi commented Aug 9, 2023

To add on top of my answer, it's also a bit linked to the discussion started in #6796
As shared, I believe stale and waiting PRs are not good, there should always be a target branch for merging a PR.
Currently the only active branch is master, that corresponds to 2.6 AFAICT.
In a situation with 2 or 3 active branches (bugfix only branch, active/new features only branch, and optionally a next work branch), I would have said that all my recent PRs about depreciation removal, that are neither about bugfix neither about an end user new feature, would have better fit in the active/new feature branch if no next branch, or the next branch.

@Kdecherf
Copy link
Member

Kdecherf commented Aug 9, 2023

To me it's a dependency refactoring, so it could fit in 2.6, but up to you :)

Fine with it, will review it and target 2.6.3 then. Thanks

@Kdecherf Kdecherf added this to the 2.6.3 milestone Aug 9, 2023
Kdecherf
Kdecherf approved these changes Aug 9, 2023
View reviewed changes
Copy link
Member

@Kdecherf Kdecherf 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, could you rebase?

@yguedidi
Copy link
Contributor Author

yguedidi commented Aug 9, 2023

Thanks! Sure I will in about 30min

@yguedidi
Copy link
Contributor Author

yguedidi commented Aug 9, 2023

@Kdecherf rebased on latest master

@Kdecherf Kdecherf merged commit 89ca32e into wallabag:master Aug 11, 2023
19 checks passed
@yguedidi yguedidi deleted the split-symfony/symfony branch August 11, 2023 20:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nicosomb nicosomb nicosomb approved these changes

@Kdecherf Kdecherf Kdecherf approved these changes

@j0k3r j0k3r Awaiting requested review from j0k3r

Assignees
No one assigned
Labels
Ready for review
Projects
None yet
Milestone
2.6.3
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@yguedidi @Kdecherf @nicosomb