Skip to content

Conversation

@ntsirintanis
Copy link
Contributor

@ntsirintanis ntsirintanis commented Mar 7, 2024

Renamed document_page_partner_id to document_page_partner and ported it from 10 to 16, with intermediate MR #314

@thomaspaulb
Copy link

Pretty straightforward - it's just the adding of 1 field. Why still draft? And there's 1 pre-commit issue

@ntsirintanis ntsirintanis force-pushed the 16.0-mig-document_page_partner branch 3 times, most recently from 1fe4abb to e39c0a3 Compare March 7, 2024 15:45
@ntsirintanis ntsirintanis marked this pull request as ready for review March 7, 2024 15:47
@thomaspaulb
Copy link

@ntsirintanis Somehow the commits seem a bit strange - the pre-commit commit removes all the files, whereas the migration commit adds them all again under a new module name. It would be better if perhaps there's a separate "rename" commit, which renames the module, after which the pre-commit is run. Or, the renaming can be included in the migration to 16.0.

Copy link
Member

@flotho flotho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks
Expected behaviour

@ntsirintanis ntsirintanis force-pushed the 16.0-mig-document_page_partner branch from e39c0a3 to 0b6fd9e Compare March 8, 2024 08:34
@thomaspaulb
Copy link

@ntsirintanis Still, I don't understand the commit structure.

  • document_page_partner_id is added
  • document_page_partner is added (Danny's copy)
  • pre-commit is run, which also includes removal of document_page_partner_id
  • document_page_partner has some migration stuff

I understand how this came to be, because you based yourself on #314, who made the original choice of not renaming but copying.

But in this way you lose commit history and you can't blame back anymore to the source of a commit.

If you're going to do it in this way, you might as well remove the commits related to document_page_partner_id completely from the history, because they carry no value anymore. But better of course is to git-commit the actual rename, so as to keep history. You could do something like:

cp -R document_page_partner /tmp
git reset HEAD~3
git mv document_page_partner_id document_page_partner
pre-commit run -A
git commit -A -m "[UPD] pre-commit"
rm -Rf document_page_partner
cp -R /tmp/document_page_partner .
git add .
pre-commit run
git commit -m "[MIG] document_page_partner to 16.0"

@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

@HaraldPanten
Copy link
Contributor

/ocabot migration document_page_partner

@OCA-git-bot OCA-git-bot added this to the 16.0 milestone Mar 28, 2024
@OCA-git-bot OCA-git-bot mentioned this pull request Mar 28, 2024
12 tasks
@HaraldPanten
Copy link
Contributor

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

On my way to merge this fine PR!
Prepared branch 16.0-ocabot-merge-pr-465-by-HaraldPanten-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 04f6a3e into OCA:16.0 Mar 28, 2024
@OCA-git-bot
Copy link
Contributor

@HaraldPanten The merge process could not be finalized, because command twine upload --repository-url https://upload.pypi.org/legacy/ -u __token__ odoo_addon_document_page_partner-16.0.1.0.0.1-py3-none-any.whl failed with output:

Uploading distributions to https://upload.pypi.org/legacy/
Uploading odoo_addon_document_page_partner-16.0.1.0.0.1-py3-none-any.whl
�[?25l
�[2K�[35m  0%�[0m �[90m━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━�[0m �[32m0.0/29.0 kB�[0m • �[36m--:--�[0m • �[31m?�[0m
�[2K�[35m  0%�[0m �[90m━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━�[0m �[32m0.0/29.0 kB�[0m • �[36m--:--�[0m • �[31m?�[0m
�[2K�[35m  0%�[0m �[90m━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━�[0m �[32m0.0/29.0 kB�[0m • �[36m--:--�[0m • �[31m?�[0m
�[2K�[35m100%�[0m �[90m━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━�[0m �[32m29.0/29.0 kB�[0m • �[33m00:00�[0m • �[31m68.9 MB/s�[0m
�[2K�[35m100%�[0m �[90m━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━�[0m �[32m29.0/29.0 kB�[0m • �[33m00:00�[0m • �[31m68.9 MB/s�[0m
�[2K�[35m100%�[0m �[90m━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━�[0m �[32m29.0/29.0 kB�[0m • �[33m00:00�[0m • �[31m68.9 MB/s�[0m
�[?25h�[33mWARNING �[0m Error during upload. Retry with the --verbose option for more details. 
�[31mERROR   �[0m HTTPError: 403 Forbidden from https://upload.pypi.org/legacy/          
         New uploads are temporarily disabled. See                              
         https://pypi.org/help/#admin-intervention for more information.        

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants