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.

Sign up for GitHub

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

Jump to bottom

[16.0][MIG] partner_helper #1412

Closed
wants to merge 26 commits into from
Closed

[16.0][MIG] partner_helper #1412

wants to merge 26 commits into from

Conversation

florian-dacosta
Copy link
Contributor

standard migraton, works without changes

@bealdav

bealdav and others added 26 commits November 23, 2022 17:45
@bealdav @florian-dacosta
[ADD] partner_helper module
8b7a593
@gurneyalex @florian-dacosta
Add OCA as author of OCA addons
e39b61a 
In order to get visibility on https://www.odoo.com/apps the OCA board has
decided to add the OCA as author of all the addons maintained as part of the
association.
@yvaucher @florian-dacosta
Add missing default oca icons
63c8596
@oca-transbot @florian-dacosta
OCA Transbot updated translations from Transifex
45f1e71
@oca-transbot @florian-dacosta
OCA Transbot updated translations from Transifex
65c50c9
@sbidoul @florian-dacosta
[UPD] prefix versions with 8.0
4393a28
@oca-transbot @florian-dacosta
OCA Transbot updated translations from Transifex
69d1116
@pedrobaeza @florian-dacosta
[MIG] Make modules uninstallable
60ead3c
@sbidoul @florian-dacosta
[FIX] remove en.po that was erroneously created by transbot
9b4b5da
@pedrobaeza @florian-dacosta
[MIG] Rename manifest files
bc42288
@EBII @florian-dacosta
[MIG] partner_helper: Migration to 10.0 (OCA#386)
351f910 
* Migrate to V10
* Tests
* Remove method args not necessary with new api
@oca-transbot @florian-dacosta
OCA Transbot updated translations from Transifex
7a223aa
@bealdav @florian-dacosta
MIGR partner_helper: to v12
289b6d5
@OCA-git-bot @florian-dacosta
[UPD] README.rst
3231951
@oca-travis @florian-dacosta
[UPD] Update partner_helper.pot
9f1f81b
@oca-transbot @florian-dacosta
Update translation files
cbe6fab 
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: partner-contact-12.0/partner-contact-12.0-partner_helper
Translate-URL: https://translation.odoo-community.org/projects/partner-contact-12-0/partner-contact-12-0-partner_helper/
@OCA-git-bot @florian-dacosta
[UPD] README.rst
1eacbd3
@badbole @florian-dacosta
Translated using Weblate (Croatian)
fc4b9fc 
Currently translated at 100.0% (1 of 1 strings)

Translation: partner-contact-12.0/partner-contact-12.0-partner_helper
Translate-URL: https://translation.odoo-community.org/projects/partner-contact-12-0/partner-contact-12-0-partner_helper/hr/
@florian-dacosta
[IMP] partner_helper: black, isort, prettier
8adabd2
@florian-dacosta
[MIG] partner_helper: Migration to 14.0
e8d3fe2
@oca-travis @florian-dacosta
[UPD] Update partner_helper.pot
8f753bf
@OCA-git-bot @florian-dacosta
[UPD] README.rst
64805d7
@oca-transbot @florian-dacosta
Update translation files
cccfc98 
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: partner-contact-14.0/partner-contact-14.0-partner_helper
Translate-URL: https://translation.odoo-community.org/projects/partner-contact-14-0/partner-contact-14-0-partner_helper/
@florian-dacosta
Translated using Weblate (Portuguese (Brazil))
a2d17a0 
Currently translated at 100.0% (1 of 1 strings)

Translation: partner-contact-14.0/partner-contact-14.0-partner_helper
Translate-URL: https://translation.odoo-community.org/projects/partner-contact-14-0/partner-contact-14-0-partner_helper/pt_BR/
@florian-dacosta
[IMP] partner_contact : precommit stuff
c592ddf
@florian-dacosta
[MIG] Migrate partner_helper to v16
f16aaec
bealdav
bealdav approved these changes Nov 24, 2022
View reviewed changes
Copy link
Member

@bealdav bealdav left a comment

Choose a reason for hiding this comment

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

LGTM code review, it seems no changes in Odoo can break things

@rousseldenis
Copy link
Contributor

/ocabot migration partner_helper

@OCA-git-bot OCA-git-bot added this to the 16.0 milestone Dec 15, 2022
@OCA-git-bot OCA-git-bot mentioned this pull request Dec 15, 2022
Open
60 tasks
@rousseldenis
Copy link
Contributor

@florian-dacosta Could you maybe add tests for both uncovered code cases ?

image

@github-actions
Copy link

There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days.
If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.

@github-actions github-actions bot added the stale PR/Issue without recent activity, it'll be soon closed automatically. label Apr 16, 2023
@github-actions github-actions bot closed this May 21, 2023
@bealdav
Copy link
Member

bealdav commented May 22, 2023

@rousseldenis might we consider to only add tests request in the roadmap section here for the next migration considering that this piece of code is not so complex, then no so critical, because completely independent of odoo changes (except number of street fields ;-) )

It's also used in production since several years.

It's important that makes things happen with trivial PR and make security barriers proportional to risks, I suppose.

@florian-dacosta are you ok with that ?

@florian-dacosta
Copy link
Contributor Author

florian-dacosta commented May 22, 2023 •
edited
Loading

Adding test on this module is on my todolist but it is not in the top of it, that's why I did not do it yet !

I agree, it is a pity to block the migration in that case but also if we just add it in the roadmap, there are chance we never add tests ...
So I have no strong opinion on this
I'll reopen it though, since I will eventually add the test one day, hopefully before the next 4 months!
@rousseldenis I can't reopen it, is it normal? Should I create a new one ?

@rousseldenis rousseldenis reopened this May 22, 2023
@rousseldenis
Copy link
Contributor

Adding test on this module is on my todolist but it is not in the top of it, that's why I did not do it yet !

I agree, it is a pity to block the migration in that case but also if we just add it in the roadmap, there are chance we never add tests ... So I have no strong opinion on this I'll reopen it though, since I will eventually add the test one day, hopefully before the next 4 months! @rousseldenis I can't reopen it, is it normal? Should I create a new one ?

Done

@rousseldenis
Copy link
Contributor

@florian-dacosta @bealdav I'm wondering why partner_helper ?

As this module implements an adress splitting, it should be renamed by partner_address_split or something like this.
IMHO, this is not a good idea to have modules that have such names as they don't represent what module does in reality. Those 'fourre-tout' modules should be avoided. Moreover, it should help also people finding it

@bealdav
Copy link
Member

bealdav commented May 22, 2023

nice, partner_address_split seems a really good name. in that case, we could add a log warning to inform of the new name here and make a new PR with the new name.

@github-actions github-actions bot removed the stale PR/Issue without recent activity, it'll be soon closed automatically. label May 28, 2023
@florian-dacosta florian-dacosta mentioned this pull request Jun 15, 2023
Merged
@florian-dacosta
Copy link
Contributor Author

Replaced by #1543

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

@bealdav bealdav bealdav approved these changes

Assignees
No one assigned
Labels
migration
Projects
None yet
Milestone
16.0
Development

Successfully merging this pull request may close these issues.