[16.0][MIG] base_multi_image

[IJOL]

@thomaspaulb Here it is, thanks for the interest and quick turnaround is much appreciated

[thomaspaulb]

@IJOL pre-commit is failing

[IJOL]

@IJOL pre-commit is failing

Yeahh seen it, not really very sure what this means, and how i can solve it, need some help to iron that..

[thomaspaulb]

@IJOL Another thing - there was a migration done before in https://github.com/OCA/server-tools/pull/2618, and as a result someone posted a lot of suggestions as a comment, as well as a branch where he fixed those things.

Would it be possible for you to check if some of those comments apply for this PR as well?

[thomaspaulb]

Yeahh seen it, not really very sure what this means, and how i can solve it, need some help to iron that..

Most of the time, you can just run pre-commit run -a on your local and git add ., then push again.

I think it would help in this case as well, it's just some files that you forgot to push:

[IJOL]

@IJOL Another thing - there was a migration done before in #2618, and as a result someone posted a lot of suggestions as a comment, as well as a branch where he fixed those things.

Would it be possible for you to check if some of those comments apply for this PR as well?

I'm a little confused, so there are 2 competing PR sort of ? my approach was a little naive, saw the dead PR and no comments, so assumed the code was reviewed and approved was simply a matter of reposting the whole thing, but obviously there is much more to it than I initially saw..

[thomaspaulb]

@IJOL You added the files in base_multi_image, but as you can see in the precommit log, same error persist. I think you should do a cd .. and a git add . again, to add also the remaining uncommitted files that were generated by pre-commit ...

[IJOL]

@IJOL Another thing - there was a migration done before in #2618, and as a result someone posted a lot of suggestions as a comment, as well as a branch where he fixed those things.

Would it be possible for you to check if some of those comments apply for this PR as well?

I seen the code you pointed, i'm i think it is more mature, than what i posted initially i'm doing another force push integrating the whole thing in minutes, dont merge yet

[thomaspaulb]

Yes I just looked at it as well, basically it's this commit that needs to be cherry-picked to the work here, conflicts resolved, and then pre-commit run again and the errors fixed. And then to do a new functional test. It's a bit of work, but of course the community would be grateful if you can do it.

[pedrobaeza]

Thanks for the contribution.

Please preserve commit history following technical method explained in https://github.com/OCA/maintainer-tools/wiki/Migration-to-version-16.0.

If the jump is between several versions, you have to modify the source branch in the main command to accommodate it to this circumstance.

[IJOL]

I'm done i think, i need to do something alike to product-attribute/product_multi_image to test the whole thing

[pedrobaeza]

This is not valid while it doesn't preserve the commit history

[IJOL]

This is not valid while it doesn't preserve the commit history

Need some help to overcome this last hurdle, but after reading the docs you pointed previously I dont know which step i missed, well i missed most of them for sure, but i'm asking for a quick way if any to solve it..

[pedrobaeza]

You need to start from scratch (but saving the final work) and follow strictly the technical steps of the guide.

[thomaspaulb]

@IJOL Yeah Pedro is right.

First off, awesome that you did all this and are doing now a functional test together with product_multi_image, that's a lot of valuable work.

To get this merged however, there's one more OCA hoop to jump through and that's preserving the commit history. In commands, that would look something like this:

cp -R base_multi_image /tmp/base_multi_image  # make a backup of all that you did now to the code
git reset --hard HEAD~1  # undo your work
git cherry-pick a-number-of-commits-that-contain-the-history-of-the-module-including-the-new-changes
rm -Rf base_multi_image
cp -R /tmp/base_multi_image base_multi_image
cd ..
git add .
git commit -a -m "[FIX] pre-commit"  # commit the changes you made to make it work from that point on

For the commits to cherry-pick, you could look here:

https://github.com/odof/server-tools/commits/16.0-base_multi_image

5f36b6910fc5e0ef3fcee4c4ba4f5cf6c3ac1d4c..c53c05fd5931f267e803cc07891d0845344713cc could work.

Or, what may be easier, is that you just checkout his branch, and then make your modifications again from there.

[thomaspaulb]

Then again yes, you can also follow the steps in the guide again. That would also work, but then you have to commit odof's work on top.

[IJOL]

testing the whole thing RN, will make some more commits here and in product_multi_image, i'll try to have the commit history clean so expect more force push in both sides

[IJOL]

Found hard showstopper in product_multi_image, and a worrisome one, its seems that the image_ids field that should be inherited from base_multi_image.owner, in https://github.com/BITVAX/product-attribute/blob/05b981993bf1c1ddb63be2ecd9f46c39cce4cb43/product_multi_image/models/product_template.py didnt exists in the post_init_hook already, any suggestion to keep the thing going? I know this should in the other repo, but they are actually connected so here if not directed otherwise

[thomaspaulb]

First off we still have a minor problem in your migration commit because you've taken the changes from @clb-openfire but committed them under your own name, I guess a quick fix would be to git commit --amend and attribute him instead for the whole thing - or git reset --hard HEAD~1 and then commit his stuff first and your changes after.

About the showstopper - how do you mean that the image_ids field does not exist in the post init hook? In the migrated version of base_multi_image that we're currently looking at, the image_ids field is still defined in the Owner model. What error do you get?

[IJOL]

@clb-openfire but committed them under your own name, I guess a quick fix would be to git commit --amend and attribute him instead for the whole thing - or git reset --hard HEAD~1 and then commit his stuff first and your changes after.

I wil go for the amend is easiest, how i do it? simply add the handle for Cedric, in the commit message "Authored by @clb-openfire" or how ?

About the showstopper - how do you mean that the image_ids field does not exist in the post init hook? In the migrated version of base_multi_image that we're currently looking at, the image_ids field is still defined in the Owner model. What error do you get?

The hooks are called from product_multi_image, and in the hook context a post_init_hook it seems that the model is not already patched or something, because when the hook tries to add an image through the image_ids field and spit "field not found" that should exist as we are in the post init hook, but as it's a one2many field i will change the hook to add directly to the image table, and see if at the other side continues being a problem,

[thomaspaulb]

As for reauthoring, I think it's just a matter of typing git commit --amend and then editing the below part to match with the name, email and date of Cedric's commit:

[thomaspaulb]

Sounds strange, maybe also post the error here maybe it has another cause.

[IJOL]

I'm done here and in product_multi_image need to poke the maintainer there

[IJOL]

Is there anything I could do to have this merged ASAP?

[clb-openfire]

It looks good to me. Thanks for all that work you've done and sorry I didn't post directly a PR at the time and saved you the trouble.

[thomaspaulb]

/ocabot migration base_multi_image

[thomaspaulb]

Treating clb's approval as valid also, merging so that https://github.com/OCA/product-attribute/pull/1707 can go green

[thomaspaulb]

/ocabot merge nobump

[OCA-git-bot]

What a great day to merge this nice PR. Let's do it! Prepared branch 16.0-ocabot-merge-pr-3010-by-thomaspaulb-bump-nobump, awaiting test results.

[thomaspaulb]

@IJOL Sorry for the delay, unfortunately these things take time. The only way to really speed things up is to try and do "review trading" with other programmers so that you review each other's code and both will benefit

[OCA-git-bot]

Congratulations, your PR was merged at f24cae89d4222e8496818dc2e320e5cedb0ed15e. Thanks a lot for contributing to OCA. ❤️