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 SonataAdmin master branch #1073

Merged
merged 9 commits into from
Sep 1, 2020
Merged

Use SonataAdmin master branch #1073

merged 9 commits into from
Sep 1, 2020

Conversation

VincentLanglet
Copy link
Member

@VincentLanglet VincentLanglet commented Jul 13, 2020 •
edited

Subject

I am targeting this branch, because WAZAAAA.

@VincentLanglet VincentLanglet mentioned this pull request Jul 13, 2020
Merged
wbloszyk
wbloszyk requested changes Jul 13, 2020
View reviewed changes
composer.json Show resolved Hide resolved
src/Admin/FieldDescription.php Show resolved Hide resolved
@VincentLanglet VincentLanglet force-pushed the wipMaster branch 4 times, most recently from 30bac96 to edd827d Compare July 17, 2020 07:13
@VincentLanglet VincentLanglet marked this pull request as ready for review July 17, 2020 23:19
@VincentLanglet VincentLanglet requested a review from a team July 17, 2020 23:20
phansys
phansys requested changes Jul 18, 2020
View reviewed changes
composer.json Outdated Show resolved Hide resolved
jordisala1991
jordisala1991 reviewed Jul 18, 2020
View reviewed changes
.travis.yml Outdated Show resolved Hide resolved
@jordisala1991
Copy link
Member

I dont know if we should do it here, on another PR or not at all but should we change this to v2?

"provide": { "sonata-project/admin-bundle-persistency-layer": "1.0.0" },

This is what connects Sonata Admin with ORM too. (to have multiple persistence bundles)

@VincentLanglet
Copy link
Member Author

I dont know if we should do it here, on another PR or not at all but should we change this to v2?

"provide": { "sonata-project/admin-bundle-persistency-layer": "1.0.0" },

This is what connects Sonata Admin with ORM too. (to have multiple persistence bundles)

Can't say, I don't know how this works...

@VincentLanglet VincentLanglet mentioned this pull request Jul 25, 2020
Merged
@SonataCI
Copy link
Collaborator

Could you please rebase your PR and fix merge conflicts?

@jordisala1991
Copy link
Member

We should be able to rebase this and merge it with dev kit changes right?

@jordisala1991
Copy link
Member

The new dev-kit PR needs this one first, Can you rebase?

@VincentLanglet
Copy link
Member Author

The new dev-kit PR needs this one first, Can you rebase?

I rebase, and took the code from the dev-kit PR to test it.

But dev-kit fail for another reason.
There is a build with dev-master.* which doesn't exist. We should update the dev-kit.

@jordisala1991
Copy link
Member

Seems like dev-kit is not prepared o handle anything different from a number here... because it does 3.*

@VincentLanglet
Copy link
Member Author

Seems like dev-kit is not prepared o handle anything different from a number here... because it does 3.*

Yes, we should update the .travis.yml.twig to handle dev-master.

@VincentLanglet
Copy link
Member Author

I tried this: sonata-project/dev-kit#797

@VincentLanglet VincentLanglet mentioned this pull request Aug 1, 2020
Merged
@VincentLanglet VincentLanglet mentioned this pull request Aug 16, 2020
Closed
@SonataCI
Copy link
Collaborator

Could you please rebase your PR and fix merge conflicts?

@VincentLanglet VincentLanglet force-pushed the wipMaster branch 5 times, most recently from bde8eb9 to c3887f3 Compare August 16, 2020 11:52
@codecov-commenter
Copy link

codecov-commenter commented Aug 31, 2020 •
edited

Codecov Report

Merging #1073 into master will increase coverage by 1.84%.
The diff coverage is 74.38%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1073      +/-   ##
============================================
+ Coverage     76.05%   77.90%   +1.84%     
- Complexity      600      601       +1     
============================================
  Files            39       39              
  Lines          1587     1498      -89     
============================================
- Hits           1207     1167      -40     
+ Misses          380      331      -49
Impacted Files Coverage Δ Complexity Δ
src/Filter/DateRangeFilter.php 0.00% <0.00%> (ø) 1.00 <1.00> (ø)
src/Filter/ModelAutocompleteFilter.php 0.00% <0.00%> (ø) 17.00 <11.00> (ø)
src/Datagrid/Pager.php 57.89% <50.00%> (-3.09%) 15.00 <4.00> (-1.00)
src/Filter/NumberFilter.php 70.58% <50.00%> (ø) 8.00 <2.00> (ø)
src/Filter/StringFilter.php 85.71% <50.00%> (ø) 17.00 <2.00> (ø)
src/Model/ModelManager.php 65.29% <55.55%> (+3.63%) 100.00 <72.00> (-2.00) ⬆️
src/Filter/AbstractDateFilter.php 69.56% <66.66%> (ø) 38.00 <7.00> (ø)
src/Admin/FieldDescription.php 74.35% <75.00%> (-2.39%) 22.00 <11.00> (-1.00)
src/Filter/ModelFilter.php 68.00% <75.00%> (ø) 20.00 <9.00> (ø)
src/Datagrid/ProxyQuery.php 86.86% <78.57%> (ø) 52.00 <44.00> (+7.00)
... and 20 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cec98d5...297b7af. Read the comment docs.

@VincentLanglet
Copy link
Member Author

@OskarStark It should be ok now.

@sonata-project/contributors This is RTM IMHO and it currently blocks the dev-kit.

jordisala1991
jordisala1991 previously approved these changes Sep 1, 2020
View reviewed changes
@VincentLanglet
Copy link
Member Author

@phansys Your approval is required :)

@OskarStark
Copy link
Member

We should first upmerge 3.x to master and then rebase this PR on master

@VincentLanglet VincentLanglet mentioned this pull request Sep 1, 2020
Closed
@phansys
Copy link
Member

phansys commented Sep 1, 2020

@phansys Your approval is required :)

Hey! I'll be very busy until this thursday, so I'm not available to give a deep review. At first sight, the only question I'm still have is why are we using dev-master instead of the extra.branch-alias definition, if the purpose is to test the future release from 4.x.

@VincentLanglet
Copy link
Member Author

@phansys You mean this: d28e6cc ?

@phansys phansys added the major label Sep 1, 2020
@jordisala1991
Copy link
Member

jordisala1991 commented Sep 1, 2020 •
edited

I can't merge because of this message:

This branch cannot be rebased due to conflicts
Rebasing the commits of this branch on top of the base branch cannot be performed automatically due to conflicts encountered while reapplying the individual commits from the head branch.

can you?

@VincentLanglet VincentLanglet merged commit a989ee3 into sonata-project:master Sep 1, 2020
@VincentLanglet
Copy link
Member Author

I had no issue to merge this

@VincentLanglet VincentLanglet deleted the wipMaster branch September 1, 2020 15:20
@VincentLanglet VincentLanglet mentioned this pull request Sep 2, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@john-dufrene-dev john-dufrene-dev john-dufrene-dev left review comments

@jordisala1991 jordisala1991 jordisala1991 approved these changes

@phansys phansys phansys approved these changes

@wbloszyk wbloszyk wbloszyk left review comments

@OskarStark OskarStark Awaiting requested review from OskarStark

Assignees
No one assigned
Labels
major
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@VincentLanglet @jordisala1991 @SonataCI @john-dufrene-dev @OskarStark @codecov-commenter @phansys @wbloszyk