Closed jordisala1991 closed 2 years ago
Merging #32 (e3d7e12) into master (217da01) will decrease coverage by
2.34%
. The diff coverage is91.66%
.
@@ Coverage Diff @@
## master #32 +/- ##
============================================
- Coverage 68.46% 66.11% -2.35%
+ Complexity 41 39 -2
============================================
Files 7 6 -1
Lines 130 121 -9
============================================
- Hits 89 80 -9
Misses 41 41
Impacted Files | Coverage Δ | |
---|---|---|
src/Form/Manipulator/DoctrineORMManipulator.php | 78.26% <91.66%> (+0.48%) |
:arrow_up: |
src/ObjectInfo/DoctrineORMInfo.php | 82.14% <0.00%> (-1.73%) |
:arrow_down: |
src/DependencyInjection/Configuration.php | 0.00% <0.00%> (ø) |
|
src/DependencyInjection/A2lixAutoFormExtension.php | 0.00% <0.00%> (ø) |
|
src/A2lixAutoFormBundle.php |
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 217da01...e3d7e12. Read the comment docs.
Do you have time for a review @webda2l ?
Thank you for your time :)
Sound good, thanks @jordisala1991 !
This PR tries to address the issue with the fields order for auto Form.
The problem (related to #11) is because by default the orm field order is not related to the class property order or the table field order, so relying on it is not recommended in any way. You can see:
https://stackoverflow.com/questions/24148568/doctrineschemaupdate-doesnt-respect-column-order
With this changes, the code is simpler, the test suite passes and also we are now relying on the order of the fields provided, without any extra configuration.
Closes #11