search

ForgeFlow / ddmrp

DDMRP
GNU Affero General Public License v3.0
14 stars 17 forks source link

Migration 11.0 #47

Closed guewen closed 6 years ago

guewen commented 6 years ago

Will move to https://github.com/OCA/ddmrp/pull/3

guewen commented 6 years ago

Travis is not running the tests because they don't pass at this point :laughing:

guewen commented 6 years ago

Rebased from 10.0. All tests pass!

guewen commented 6 years ago

Proposed on https://github.com/OCA/ddmrp/pull/3 but I'm not sure if we want to work here first and extract only when we're sure we're good or move early to OCA. As there are changes on both 10.0 and 11.0 these days, I think it'd be easier to only work here and open the OCA PR later.

guewen commented 6 years ago

I rebased from 10.0 to get the last changes (https://github.com/Eficent/ddmrp/commit/a88a005d2e3aae1a4fd664243e58ce691dc94b21, ...)

LoisRForgeFlow commented 6 years ago

Just code reviewed the tests and they look good. Just one comment: dlt should be removed from all the creating dicts for buffers as it is now computed, e.g. example

LoisRForgeFlow commented 6 years ago

@guewen There are conflicts here, could you have a look?

grindtildeath commented 6 years ago

@guewen @simahawk I attended most of the changes asked. @lreficent Don't know if the conflicts you mentioned have been solved in the meanwhile, but it looks ok to me at the moment.

LoisRForgeFlow commented 6 years ago

@grindtildeath ~the conflicts haven't been solved, I cannot merge due to them~

EDIT: Nevermind, I was to quick answering. I can use any of the other methods to merge :smile:

LoisRForgeFlow commented 6 years ago

I will try to review and merge this week.

LoisRForgeFlow commented 6 years ago

@guewen I proposed you a PR: https://github.com/guewen/ddmrp/pull/1

Once you merge that one I think we are ready to move forward.

guewen commented 6 years ago

@lreficent merged, thanks

LoisRForgeFlow commented 6 years ago

Finally merged! Thank you all for the effort! :smile:

guewen commented 6 years ago

@lreficent should I update https://github.com/OCA/ddmrp/pull/3 or is it better to wait we have all the addons?

LoisRForgeFlow commented 6 years ago

@guewen I think there is only one addon pending to merge in OCA (mrp_bom_location, it will be only a rebase when it is merged, but up to you!

guewen commented 6 years ago

I was speaking about #52, #51 and #50, maybe it's simpler to move to OCA once they have been merged as well

LoisRForgeFlow commented 6 years ago

Oh, yes, I think if you rebase them I can review and merge here next week.

JordiBForgeFlow commented 6 years ago

@guewen I agree that perhaps in this case it is better to send a single PR for all the modules in DDMRP. We know all the reviewers invoved (maxime, joël, us, raphaël, etc...) and all are very familiar with the modules.