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

[WIP] Add Asset Management #5

Merged
merged 16 commits into from Nov 14, 2015
Merged

[WIP] Add Asset Management #5

merged 16 commits into from Nov 14, 2015

Conversation

pedrocasi
Copy link

No description provided.

@oca-clabot
Copy link

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:

  • @pcs-sossia (login unknown in OCA database)

Appreciation of efforts,
OCA CLAbot

@pedrocasi
Copy link
Author

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
Copy link
Sponsor Member

max3903 commented Jun 25, 2015

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

"author": "Sossia, P. Baeza, "
"Odoo Community Association (OCA)",
'summary': 'Implementa os requisitos legais para gestão de Imobilizado',
"description": """
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

The description key is ignored when a READ me is present, as is the case, so it can be removed.

@dreispt
Copy link
Sponsor Member

dreispt commented Jun 26, 2015

@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
Copy link
Sponsor Member

dreispt commented Jun 26, 2015

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

@pedrocasi
Copy link
Author

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
Copy link
Member

We are keeping our l10n_es_account_asset adaptation because it's very lightweight, but in the future the plans are to make adaptations over the new account_asset_management 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
Copy link
Sponsor Member

dreispt commented Jul 3, 2015

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

@pedrobaeza
Copy link
Member

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

@dreispt
Copy link
Sponsor Member

dreispt commented Jul 3, 2015

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

@pedrocasi
Copy link
Author

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
Copy link
Sponsor Member

dreispt commented Jul 3, 2015

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

@dreispt dreispt changed the title Initial submission to OCA [WIP] Add Asset Management Aug 28, 2015
@oca-clabot
Copy link

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

  • @pcs-sossia

Appreciation of efforts,
OCA CLAbot

@dreispt
Copy link
Sponsor Member

dreispt commented Sep 29, 2015

@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

@@ -0,0 +1,38 @@
# -*- encoding: utf-8 -*-
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

s/encoding/coding

@pedrocasi
Copy link
Author

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 ...


{
"name": "Portugal - IVA",
"version": "0.2",
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

8.0.0.2.0

# sale-workflow https://github.com/OCA/sale-workflow branchname

account_asset_management
account-financial-tools
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

You just need the line with the repo name account-financial-tools.
Remove the line with module name account_asset_management - that's the reason the CI build is failing.

@dreispt
Copy link
Sponsor Member

dreispt commented Nov 6, 2015

👍 Thanks for you patience and perseverance.

@dreispt
Copy link
Sponsor Member

dreispt commented Nov 6, 2015

A second review is needed now.

@dreispt
Copy link
Sponsor Member

dreispt commented Nov 9, 2015

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

dreispt added a commit that referenced this pull request Nov 14, 2015
Add Asset Management
@dreispt dreispt merged commit 900e54c into OCA:8.0 Nov 14, 2015
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.

None yet

5 participants