[10.0][MIG] Migrate external_file_location

[cmsalmeida]

This module has attachment_base_synchronize dependency, so I need to branch from existent #763

[yajo]

Great contribution, thank you!
I labeled as WIP until merged #763 then.

[cmsalmeida]

Any idea when will that happen?

[yajo]

You already asked that, but the answer is still the same: code must be reviewed and approved. If you are interested, you can review it too 😉

[cmsalmeida]

I've reviewed it

[cmsalmeida]

Why work in progress?
It's done...

[paulius-sladkevicius]

Did few tests, works perfectly 👍

[cmsalmeida]

23 days passed and it's not merged yet, any prediction?

[lasley]

@cmsalmeida - on quick glance, there are a lot of issues with this PR, including the removal of attribution. It is quite large though, so it will likely take a while for a PSC to review it. You can typically get someone to review your code by reviewing theirs', or just simply waiting for someone that cares about this module to review it.

Curious though - Why is a merge of this code relevant to your workflow?

[rvalyi]

cc @florian-dacosta free migration of our module.. may be you can review the PR?

[rvalyi]

cc @sebastienbeau

[rvalyi]

cc @bealdav

[rvalyi]

@florian-dacosta I see here that you removed some contributors in the README during the v9 migration bac7db7#diff-0ced2de67fc0b7063512da23a694dcc0L67
was it intentional, is there a good reason for it? cc @sebastienbeau @jgrandguillaume

@cmsalmeida I see that your work is mostly the v9 to v10 migration in this commit bac7db7#diff-0ced2de67fc0b7063512da23a694dcc0L67
That is switching 'openerp' for 'odoo', re-indenting the XMLs and doing a few minor view adjustments. That is probably fine to be added in the contributors section in the README for this work, but putting your company in the module author section is probably a bit exaggerate no? Please don't take it personaly, but I mean if everybody who re-indent a file in some migration were given the module creation credit just like the one who created an OCA module, we may have issues for giving visibility to the companies investing heavily in the OCA R&D...

[florian-dacosta]

@rvalyi
For the contributors removal, my guess is that the contributor list was copied from another module and it was nonsense for this one, as far as I know, it is 100% from akretion, but if I am wrong, I have no problem for putting contributors back. For Sebastien, I am not sure, let's see what is says!
For the review, yes, it is planned, I was kind of waiting attachment_base_synchronize to be merge but I'll do a full review/test soon

[bealdav]

Hi all,

Joel Grand Guillaume, was mentionned because we used some good practice in its previous module.
but no commit from himself on this module.

I think remembering it's the same case for initos (some really little part of the code copy/paste)

There is no precise guideline in OCA about contributors mentionned, then we shouldn't block this PR.

@cmsalmeida if you want to go fast, you can reintroduce these authors, then nobody can contest.

[sebastienbeau]

Regarding the contributor
Init os was here because we take some code from here see discusion here : initOS/connector-interfaces#1
I have done the first version of this project (was on launchpad a long time ago) and we take some code from this project.

In any case the most important THANK FOR YOUR WORK !
@florian-dacosta can you do a review so we can merge is work.

[cmsalmeida]

No problem for me to update that if it is such a problem, and delaying merging.
Thank you all.

[cmsalmeida]

It give error on:
"The command "travis_install_nightly" failed and exited with 1 during ."
So it's not an error related to this modules.
How to trigger the tests again?

[lasley]

@cmsalmeida - I restarted the Travis build

[cmsalmeida]

@lasley thank you

[lasley]

@cmsalmeida - The Travis build is failing due to OCA/maintainer-quality-tools#459 - we can trust the Runbot for the moment & Travis will be cleared soon.

[lasley]

Issue fixed, rebuilding

[florian-dacosta]

Seems good to me
Code review + some tests

[lasley]

@cmsalmeida - now that #763 has been merged, can you please rebase so that those commits are removed from here? Will make it a bit easier to review IMO

[bealdav]

+1

[cmsalmeida]

something went terrible wrong, created new #926

[eugen-don]

@cmsalmeida
Hi, are still working on this?
Id like to install a moduloe that depends on this particular one... Is there any way for me to help out?