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

Rename web_pwa -> pwa #1612

Closed
wants to merge 1 commit into from
Closed

Rename web_pwa -> pwa #1612

wants to merge 1 commit into from

Conversation

eLBati
Copy link
Member

@eLBati eLBati commented Jun 4, 2020

@eLBati eLBati mentioned this pull request Jun 4, 2020
Copy link
Member

@pedrobaeza pedrobaeza left a comment

Choose a reason for hiding this comment

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

I'm against this change as said. And the name is not valid, as we can have also portal_pwa as stated in the roadmap.

Add only the symbolic link and we'll see if it's taken by Odoo Apps.

@pedrobaeza pedrobaeza added this to the 12.0 milestone Jun 4, 2020
@eLBati eLBati mentioned this pull request Jun 4, 2020
@eLBati
Copy link
Member Author

eLBati commented Jun 4, 2020

Ok, trying with #1613

@pedrobaeza pedrobaeza closed this Jun 4, 2020
@eLBati
Copy link
Member Author

eLBati commented Jun 4, 2020

So, let's hear what Web PSC think about this
@OCA/web-maintainers

@legalsylvain
Copy link
Contributor

legalsylvain commented Jun 4, 2020

Hi.
Well, regarding the module, the perfect name is still web_pwa.
As there is a conflict on Odoo apps store, 2 options :
A) keep the name web_pwa. Generally, it's not a big deal if a module is not present in the Odoo Apps store. In the other hand, this module is very great and take part of the OCA R&D. It's a pitty if it's not visible in the main odoo apps store. (Unfortunately, OCA apps store is not very used for the time being).
B) rename the module. I propose simply web_pwa_oca.
In that case 2 options :
B1) rename in the current V12 version
B2) rename for the future version 13+ to avoid breaking production instance.

I'm in favor of B1, because the module is available only since a few days (and not available in the appstore). so making a communication in github & twitter is enough in my opinion. but not a blocking point, I understand other point of views.

Anyway, @eLBati thanks again for this great module.

@marcelsavegnago
Copy link
Member

Considering the installations already made (my case and @pedrobaeza ), if necessary to change the module name to web_pwa_oca is it possible to create the migration script inside the module itself to make the name change? In addition, it was great that this module has no dependence on web_responsive (despite an excellent module) as it allows the use of this module with other themes without conflict.

@simahawk
Copy link
Contributor

simahawk commented Jun 5, 2020

I'm sorry for @marcelsavegnago and @pedrobaeza and whoever installed this earlier (I can feel the pain) but this is what ALPHA/BETA maturity stand for. It means it can change.

And in any case if @eLBati cares about putting his own work on odoo apps we should allow him to do it in all the ways we can. Stopping this just because few of us out of hundreds of ppl have installed it in a rush on many instances is not a good reason.

What we can do IMO is (unless we find any better way to fix this):

  1. help him choosing a better name. Being pwa, portal_pwa, mobile_pwa or whatever. Personally I don't care but this time let's make sure is not there on odoo apps yet 😄

  2. ask him KINDLY to add a migration step to make this smoother for early adopters

  3. review is work

@eLBati
Copy link
Member Author

eLBati commented Jun 5, 2020

I can add a migration script gladly.

What's the best way in this case?

  1. Add migrations/12.0.1.0.0/pre-migration.py
  2. Add the script to __init__.py file
  3. Add a descriptive step to INSTALL.rst, in case web_pwa was installed:
>>> from openupgradelib import openupgrade
>>> openupgrade.update_module_names(env.cr, [('web_pwa', 'web_pwa_oca')], merge_modules=False)
>>> env.cr.commit()

Thanks

@marcelsavegnago
Copy link
Member

marcelsavegnago commented Jun 5, 2020

@eLBati web_pwa_oca is ok for me and thanks for migration script.
I really liked the module. Thank you.

@simahawk no problem for me. I agree with you and understand that we must guarantee the visibility of the module.

@eLBati
Copy link
Member Author

eLBati commented Jun 5, 2020

Moving to #1616

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants