fopina / django-bulk-update-or-create

`bulk_update_or_create` for Django model managers
MIT License
148 stars 16 forks source link

Support for multiple match_field #6

Closed thejoeejoee closed 3 years ago

thejoeejoee commented 3 years ago
codecov-io commented 3 years ago

Codecov Report

Merging #6 (91904f8) into master (8158c1e) will increase coverage by 0.12%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master       #6      +/-   ##
==========================================
+ Coverage   98.66%   98.79%   +0.12%     
==========================================
  Files           2        2              
  Lines          75       83       +8     
==========================================
+ Hits           74       82       +8     
  Misses          1        1              
Impacted Files Coverage Ξ”
bulk_update_or_create/query.py 98.75% <100.00%> (+0.13%) :arrow_up:

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 8158c1e...91904f8. Read the comment docs.

fopina commented 3 years ago

hey @thejoeejoee still around? I think I lost track of all notifications in this small project and missed quite a few interesting ones.

Trying to catch up and your PR is quite wanted :) I've opened #19 to do few changes before merging (keeping your original commit for credits!)

Let me know if you around to do them (and keep git/PR history cleaner)

thejoeejoee commented 3 years ago

@fopina no problem, I see a lot of changes in #15 conflicting with this PR -- still, I can make it working, gimme day or two

fopina commented 3 years ago

@thejoeejoee yes, I should've started with yours and work on it instead but if you don't mind it going in #19 (and keeping that commit of yours), I'll go with it in a few as I'm resolving the conflicts at the moment

disclosure: I actually need the feature in a project πŸ•Ί

fopina commented 3 years ago

@thejoeejoee pushed resolution on #19 with a bit of refactor already if you care to take a look? rebased by mistake instead of merge so all ended up under your commit

fopina commented 3 years ago

(same) tests on #19 passed with that bit of refactor (plus rebasing).

let me know if you can just reset your local fork with that branch, amend commit and push it here, so your contribution is properly tracked (and my mess/hurry is forgotten - sorry)

I'll merge (either this PR after reset or #19) later today and make a release πŸŽ‰

Contribution very much welcome πŸ™‡

thejoeejoee commented 3 years ago

@fopina reset done and force-pushed :heavy_check_mark: thanks for your work on this repository!

fopina commented 3 years ago

thank you. really nice to need a feature on your own package and have it already implemented in a PR!