OCA / l10n-portugal

Odoo Portuguese localization
GNU Affero General Public License v3.0
4 stars 26 forks source link

[WIP] Add Asset Management #5

Closed pedrocs-exo closed 8 years ago

oca-clabot commented 9 years ago

Hey @pcs-sossia, thank you for your Pull Request.

It looks like some users haven't signed our Contributor License Agreement, yet. You can read and sign our full Contributor License Agreement here: http://odoo-community.org/page/website.cla Here is a list of the users:

Appreciation of efforts, OCA CLAbot

pedrocs-exo commented 9 years ago

Can someone pls help? I did sign the CLA both for myself and Sossia as a company. In my OCA account profile I can see pcs-sossia on the GitHub Login field. What am I missing?

max3903 commented 9 years ago

@gurneyalex Can you please help here ? Indeed, everything seems to be fine but still the bot complains. Thanks.

dreispt commented 9 years ago

@pcs-sossia Check the TravisCI log - there are a couple of lint checks to be fixed.

I can see that the logic side is strongly based on the spanish localization. I dislike copied code, since it ends up duplicating the maintenance effort.

I suggest we either: 1) Make this module depend/extend on the spanish localization one, or 2) Extract the common logic into a generic l10n_account_asset to be hosted in a common OCA repository.

What do you think @pedrobaeza ?

dreispt commented 9 years ago

Oh, I almost forgot: many thanks for contributing to the OCA @pcs-sossia !

pedrocs-exo commented 9 years ago

Regarding the runbot issue, pls note that my OCA login is OAuth based from odoo.com. And I can't login to https://runbot.odoo-community.org because there's no option for odoo.com accounts. Is that the problem? If it is, pls convert my account into a locally authenticated one and it'll do. Thx

pedrobaeza commented 9 years ago

We are keeping our _l10n_es_accountasset adaptation because it's very lightweight, but in the future the plans are to make adaptations over the new _account_assetmanagement module (https://github.com/OCA/account-financial-tools/tree/8.0/account_asset_management), that already include most of the "tricks" we have done in our adaptation, so I ask you to check this one and reconsider an adaptation over this one.

dreispt commented 9 years ago

@pedrobaeza Sorry Pedro, at least for it was not clear: are you suggesting to reuse and extend the es module?

pedrobaeza commented 9 years ago

No, I'm suggesting to extend the other one.

dreispt commented 9 years ago

Got it. Sorry @pcs-sossia but this should needs some (re)work.

pedrocs-exo commented 9 years ago

No problem. I'll take a look at the account_asset_management module and adapt my code.

Meanwhile can someone take a moment to look at the bot issue pls?

dreispt commented 9 years ago

Don't worry: @max3903 confirmed your CLA so maintainers can ignore the bot.

oca-clabot commented 9 years ago

Hey @pcs-sossia, We acknowledge that the following users have signed our Contributor License Agreement:

Appreciation of efforts, OCA CLAbot

dreispt commented 9 years ago

@pcs-sossia You need to declare the dependency on another repo addins an "oca-dependencies" file at the root of the repo. You can find a sample file here: https://github.com/OCA/maintainer-quality-tools/blob/master/sample_files/oca_dependencies.txt

@gurneyalex : @max3903 already confirmed the CLA, it seems that there's still something wrong with the cla bot

pedrocs-exo commented 9 years ago

Travis build is failing with message:

except_orm: ('Error', u"You try to install module 'l10n_pt_account_asset' that depends on module 'account_asset_management'.\nBut the latter module is not available in your system.").

Any help would be appreciated ...

dreispt commented 8 years ago

@pcs-sossia see my previous comment: add an oca_dependencies.txt file at the root, including a line with account-financial-tools I just noticed you already that, but something went wrong when TravisCI tried to clone the repo. I restarted the build to check if that's OK now.

dreispt commented 8 years ago

:+1: Thanks for you patience and perseverance.

dreispt commented 8 years ago

A second review is needed now.

dreispt commented 8 years ago

Anyone else for a review? If not, I'll be merging this.