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

[14.0] Add Missing Changes from v12 #51

Open
wants to merge 5 commits into
base: 14.0
Choose a base branch
from

Conversation

yweng8111
Copy link
Contributor

During the migration of the modules (v12 -> v14) we have made some changes that are not contained in the actual code of 14.0

@yweng8111 yweng8111 force-pushed the 14.0-sync_devel_code_from_v12 branch from 5a2a110 to 1dffc34 Compare October 9, 2024 11:45
@yweng8111 yweng8111 force-pushed the 14.0-sync_devel_code_from_v12 branch from 1dffc34 to 0556209 Compare October 9, 2024 12:14
Copy link

@mrgoetz mrgoetz left a comment

Choose a reason for hiding this comment

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

Good job! LGTM

Copy link
Contributor

@olaf-wagner olaf-wagner left a comment

Choose a reason for hiding this comment

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

Looks all good and reasonable to me.

@rousseldenis
Copy link

@yweng8111 I think this is not correct as you should keep commits as is and not changing them.

@rousseldenis
Copy link

I encourage you to use this great tool: https://github.com/OCA/oca-port

@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). 🤖

@yweng8111
Copy link
Contributor Author

yweng8111 commented Nov 6, 2024

I encourage you to use this great tool: https://github.com/OCA/oca-port

@rousseldenis
Thanks for the Tips. I've tried the usual oca migration process with git format-patch. But it doesn't work, since we reformatted the code in 12.0 after porting them to 14.0. After that we also reformatted the code in 14.0. So there are a lot of differences between 12.0 and 14.0
Even oca-port will not help me to solve this problem. I've tried it too. There are too many not fully ported commits.

$ oca-port origin/12.0 origin/14.0 rental_base --verbose
rental_base already exists on 14.0, checking PRs to port...
2 pull request(s) and 2 commit(s) w/o PR related to 'rental_base' to port from origin/12.0 to origin/14.0

1) w/o PR:
	=> Updates: 
	=> 2 commit(s) not (fully) ported
		652eb711 selectively add some files from the v12 branch
		cba265f0 initial add of several rental modules
2) PR #18 (https://github.com/OCA/vertical-rental/pull/18) 12.0 sync devel code:
	By yweng8111, merged at 2022-09-09T06:13:34Z
	=> Updates: rental_offday, rental_base, rental_product_pack, rental_check_availability, rental_pricelist_interval, rental_pricelist
	=> Not ported: rental_offday, rental_base, rental_product_pack, rental_check_availability, rental_pricelist_interval, rental_pricelist
	=> 43 commit(s) not (fully) ported
		f8f21cc9 [IMP] add group to see rental menu
		f3b9a278 [IMP] improves form view of product
		d8bf4072 [IMP] Rental UX Improvements
		850c56ed [FIX] correct Update Times , when user want to change start and end date on rental order, (issue#4516)
		bb92a3e2 [FIX] correction made when user update times on lines, unit tests added for it, (issue#4516)
		db285a95 [IMP] add some unit tests
		614a7693 [IMP] Rental UX Improvements
		b4f0ad71 [IMP] add field rental_service_copy_image for company
		0d4c938d [IMP] recursive function _create_pack_products (issue #4670)
		d34f06be [IMP] Merge products from Packs in Rental Picking Out
		051a0b9f [IMP] Update Docs of module rental_check_availability
		fd9909d4 [IMP] Update docs of module
		04462745 [IMP] Update docs of module rental_base
		cb6b3466 [IMP] refactor the dependencies in rental_base, (issue#4955)
		412cfe04 [IMP] Update docs of module rental_pricelist
		419a31b6 [IMP] Update docs of module rental_pricelist_interval
		996f47cb [IMP] Update docs of module rental_product_pack
		1d35d62e [IMP] rental_base: black, isort, prettier
		7a643785 [IMP] rental_pricelist: black, isort, prettier
		1811cff5 [IMP] rental_offday: black, isort, prettier
		059bdb83 [IMP] rental_product_pack: black, isort, prettier
		b1b802ea [IMP] rental_check_availability: black, isort, prettier
		16ea7f5d [IMP] rental_pricelist_interval: black, isort, prettier
		ea0a5a81 [IMP] improves unit test of module rental_pricelist
		cadd5b8e [IMP] add ValueError
		eca5ac96 [IMP] rental_base: black, isort, prettier
		4b2051d2 [IMP] rental_check_availability: black, isort, prettier
		abe186a6 [IMP] rental_offday: black, isort, prettier
		986cfa63 [IMP] rental_pricelist: black, isort, prettier
		813fa45f [IMP] rental_pricelist_interval: black, isort, prettier
		02210766 [IMP] rental_product_pack: black, isort, prettier
		2710d552 [IMP] delete description in manifest of module rental_base
		bf7e6693 [IMP] delete description in manifest of module rental_check_availability
		a061191b [IMP] delete description in manifest of module rental_offday
		0ea9c98b [IMP] delete description in manifest of module rental_pricelist
		e1be2a72 [IMP] delete description in manifest of module rental_pricelist_interval
		a088ef96 [IMP] delete description in manifest of module rental_product_pack
		3e9ba8f6 [IMP] Fix pylint for module rental_base
		41fd7b18 [IMP] Fix pylint for module rental_check_availability
		aebdfd65 [IMP] Fix pylint for module rental_pricelist
		35e0fbd0 [IMP] Fix pylint for module rental_pricelist_interval
		9173d629 [IMP] improves codecov for module rental_offday
		5bca4218 [IMP] disable unstable checks in unit test of rental_base
3) PR #19 (https://github.com/OCA/vertical-rental/pull/19) [IMP] add group definitions into menuitem:
	By yweng8111, merged at 2022-09-19T10:30:50Z
	=> Updates: rental_base
	=> Not ported: rental_base
	=> 1 commit(s) not (fully) ported
		a42a7330 [IMP] add group definitions into menuitem

@rousseldenis
Copy link

There are too many not fully ported commits.

And what is the issue ?

The problem is about authorship and history. With this approach, both branches will be more asynchronous from each other. We should avoid such problems.

Ideally, I would say you should first do a PR per module. This is the best approach to decouple the lever for review/approvals and fast merges.

@olaf-wagner
Copy link
Contributor

The problem is that we simply do not have time to do it the way you suggest. I'm afraid it won't be done at all then.
Yu Weng is one of the original authors of those modules and the code is in productive use at several customers. I'm sorry that we don't know the OCA tools and conventions better, but we cannot spend the time every action at the OCA repositories seems to require.

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.

5 participants