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.

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

Removed assigning properties via save() in models (as per #12317) #13945

Merged
merged 2 commits into from
Apr 6, 2019
Merged

Removed assigning properties via save() in models (as per #12317) #13945

merged 2 commits into from
Apr 6, 2019

Conversation

SidRoberts
Copy link
Contributor

Hello!

  • Type: bug fix | documentation

In raising this pull request, I confirm the following:

  • I have read and understood the Contributing Guidelines
  • I have checked that another pull request for this purpose does not exist
  • [-] I wrote some tests for this PR
  • I updated the CHANGELOG

Forgot to update some of the docblocks with the changes made in #12317 and I missed one code reference in Mvc\Model\Resultset that still tried to assign data whilst saving. I can only assume that it's in a method that isn't used very much as no one has complained since the original PR merged 2 years ago. 😮

@codecov
Copy link

codecov bot commented Apr 5, 2019

Codecov Report

Merging #13945 into 4.0.x will decrease coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##            4.0.x   #13945      +/-   ##
==========================================
- Coverage   67.64%   67.64%   -0.01%     
==========================================
  Files         469      469              
  Lines       94350    94352       +2     
==========================================
  Hits        63821    63821              
- Misses      30529    30531       +2
Impacted Files Coverage Δ
ext/phalcon/mvc/model/resultset.zep.c 46.62% <0%> (-0.2%) ⬇️
ext/phalcon/mvc/model.zep.c 66.6% <0%> (ø) ⬆️

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 4054dae...d6a27db. Read the comment docs.

sergeyklay
sergeyklay previously approved these changes Apr 5, 2019
Jeckerson
Jeckerson previously approved these changes Apr 5, 2019
@niden niden added breaks bc Functionality that breaks Backwards Compatibility enhancement Enhancement to the framework labels Apr 5, 2019
@niden
Copy link
Member

niden commented Apr 5, 2019

Need a rebase on this bud @SidRoberts

@SidRoberts SidRoberts dismissed stale reviews from Jeckerson and sergeyklay via aef147d April 5, 2019 16:52
@niden niden merged commit 729bf7d into phalcon:4.0.x Apr 6, 2019
@niden
Copy link
Member

niden commented Apr 6, 2019

Thank you @SidRoberts

@SidRoberts SidRoberts deleted the model-save branch April 7, 2019 01:56
@niden niden added the documentation Documentation required label Apr 9, 2019
@niden niden added the 4.0 label Dec 2, 2019
@niden niden removed the documentation Documentation required label Dec 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaks bc Functionality that breaks Backwards Compatibility enhancement Enhancement to the framework
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants