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

[ADD] Module to add EORI number to company form #52

Merged

luc-demeyer merged 4 commits into OCA:8.0 from lfreeke:8.0-add_eori_number

Dec 2, 2018

lfreeke commented Nov 19, 2018

No description provided.

Contributor

luc-demeyer commented Nov 19, 2018

Thanks for this contribution.
Can you fix the rst error ?
I am not sure if the 'intrastat' repo is the best place to host this module since EORI is for 'extracom' business whereas intrastat is for 'intracom' but we already host the 'product_harmonized_system' module so we can imho merge this module into this repo and think about a more generic import/export/customs kind or repo for the upcoming Odoo releases.

Member

pedrobaeza commented Nov 20, 2018

We can also rename the repository if needed.

pedrobaeza added this to the 8.0 milestone

lfreeke force-pushed the 8.0-add_eori_number branch from 4f14cad to bf0dc0f Compare

November 20, 2018 11:00

Author

lfreeke commented Nov 20, 2018

@luc-demeyer I fixed the rst errors. It seems Travis is failing because of another module.

I had trouble to find a place for this module, it doesn't really fit in other repo's too. A Custom repo is a good idea, we all have more or less the same requirements in Europe.

Contributor

luc-demeyer commented Nov 21, 2018

I'll fix the travis error via a separate PR.
What about renaming the repo to intrastat-extrastat ?

Member

pedrobaeza commented Nov 21, 2018

OK, seems reasonable

Contributor

luc-demeyer commented Nov 21, 2018

@alexis-via do you agree with rename of repo to intrastat-extrastat ?

Contributor

alexis-via commented Nov 21, 2018

I must say I don't really like the idea of renaming the project.
Let's have this module for EORI here and keep the project with it's current name. Let's keep things simple!

Member

pedrobaeza commented Nov 21, 2018

@alexis-via there's no problem in renaming. References to old name will still work without problems. We have already done it for other repositories when the content has been adjusted.

Contributor

alexis-via commented Nov 22, 2018

Yes, I know renaming the project is technically not a problem. But I think that the current name of the project should be kept because the word "intrastat" has been used in Odoo for a very long time (at least since v5, cf https://github.com/odoo/odoo/tree/5.0/addons/report_intrastat), so everybody knows what it means ! If we change the name, it will be another story !!!
And I think we can accept this module without changing the name of the project.

Member

pedrobaeza commented Nov 22, 2018

But the word will be kept. We are only adding -extrastat...

Contributor

alexis-via commented Nov 22, 2018

OK, fine :) :)

daramousk reviewed

View reviewed changes

company_eori_number/README.rst Outdated

+                  :alt: License: AGPL-3
+              ===================
+              company_eori_number

Member

daramousk Nov 22, 2018

Set a proper name for your module here.

company_eori_number/README.rst Outdated

+              Installation
+              ============
+              To install this module, you need to:

Member

daramousk Nov 22, 2018

Remove this or give proper instructions.

company_eori_number/README.rst Outdated


		To configure this module, you need to:

		#. No need for configuration

Member

daramousk Nov 22, 2018

If something does not apply, remove the section completely (ex. remove Configuration)

company_eori_number/README.rst Outdated

		:target: https://runbot.odoo-community.org/runbot//8.0


		Known issues / Roadmap

Member

daramousk Nov 22, 2018

Same here, this can be removed

daramousk reviewed

View reviewed changes

company_eori_number/README.rst Outdated

+              Usage
+              =====
+              To use this module, you need to:

Member

daramousk Nov 22, 2018

You could fill this in.

company_eori_number/README.rst Outdated

+              Contributors
+              ------------
+              * lfreeke <lfreeke@therp.nl>

Member

daramousk Nov 22, 2018

Set your name here before your email

company_eori_number/__openerp__.py Outdated

+                  "version": "8.0.1.0.0",
+                  "author": "Therp BV,Odoo Community Association (OCA)",
+                  "license": "AGPL-3",
+                  "category": "",

Member

daramousk Nov 22, 2018

Fill these in.

company_eori_number/models/res_company.py Outdated Show resolved Hide resolved


          [ADD] Module to add EORI number to company form

ee006e8

lfreeke force-pushed the 8.0-add_eori_number branch from bf0dc0f to ee006e8 Compare

November 22, 2018 16:49

luc-demeyer added 3 commits

November 25, 2018 16:52


          update README.rst, cf. comments of daramousk

06fd1ac


          Update __openerp__.py, cf. comments daramousk

7d8c258


          update res_company.py, cf. comments daramousk

ca65de0

Contributor

luc-demeyer commented Nov 25, 2018

@lfreeke @daramousk
I have updated the PR with the feedback of daramousk. Please review.

@pedrobaeza
can you rename the repo to 'intrastat-extrastat' ?

daramousk approved these changes

View reviewed changes

Member

pedrobaeza commented Nov 25, 2018

@luc-demeyer, I think @alexis-via is still not convinced about the name change.

Contributor

luc-demeyer commented Nov 25, 2018

3 days ago Alexis reacted with "OK, fine :) :)" (cf. conversation history before the comments of daramousk) hence you can go ahead.

Member

pedrobaeza commented Nov 25, 2018

Repository renamed.

lfreeke commented

View reviewed changes

Author

lfreeke left a comment

👍

Member

sbidoul commented Dec 2, 2018

@pedrobaeza when renaming repositories, one needs to think about weblate.

Member

pedrobaeza commented Dec 2, 2018

OK, let me try. Maybe we should make a Wiki page about renaming?

Member

pedrobaeza commented Dec 2, 2018

OK, as both were duplicated right now, but with exact contents, I have removed the projects with old name.

luc-demeyer merged commit fa322e0 into OCA:8.0

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