Skip to content

Conversation

@baimont
Copy link
Contributor

@baimont baimont commented Oct 4, 2022

Copy link
Member

@hugosantosred hugosantosred left a comment

Choose a reason for hiding this comment

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

Thanks for the migration. Apart from that comment looks good to me

@baimont baimont force-pushed the 16.0-mig-queue_job-bai branch from a786a5d to ed9f633 Compare October 5, 2022 09:07
@baimont baimont requested a review from hugosantosred October 5, 2022 09:08
@guewen
Copy link
Member

guewen commented Oct 11, 2022

Hi, thanks for your work!
Could you migrate test_queue_job as well?
If it is not migrated at the same time, we have no way to tell if queue_job behaves as expected as most of the tests are there.

@baimont baimont changed the title [16.0] [MIG] queue_job [16.0] [MIG] queue_job and test_queue_job Oct 11, 2022
@baimont
Copy link
Contributor Author

baimont commented Oct 11, 2022

Hi, thanks for your work! Could you migrate test_queue_job as well? If it is not migrated at the same time, we have no way to tell if queue_job behaves as expected as most of the tests are there.

Hi @guewen, done

requirements.txt Outdated
@@ -1 +1,3 @@
# generated from manifests external_dependencies
mock
Copy link
Member

Choose a reason for hiding this comment

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

@baimont can't we replace the import with from unittest import mock?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@guewen yes this is better. I adapted the commit

@baimont baimont force-pushed the 16.0-mig-queue_job-bai branch from eeba81e to 6caca1a Compare October 11, 2022 14:10
Copy link
Member

@guewen guewen left a comment

Choose a reason for hiding this comment

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

Great! Many thanks!

@guewen
Copy link
Member

guewen commented Oct 11, 2022

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 16.0-ocabot-merge-pr-460-by-guewen-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit cafc5c9 into OCA:16.0 Oct 11, 2022
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at e7c272d. Thanks a lot for contributing to OCA. ❤️

@guewen
Copy link
Member

guewen commented Oct 11, 2022

@baimont did you check if jobs were executed? I'm testing locally (I have been enthusiastic about merging 😅), but jobs aren't executed. Now, I may have botched my install

@baimont
Copy link
Contributor Author

baimont commented Oct 11, 2022

@baimont did you check if jobs were executed? I'm testing locally (I have been enthusiastic about merging 😅), but jobs aren't executed. Now, I may have botched my install

Ahahaha I always execute them myself with pytest-odoo before pushing any change, so yes

@guewen
Copy link
Member

guewen commented Oct 11, 2022

That's nice, but sorry I speak about jobs ;)

@baimont
Copy link
Contributor Author

baimont commented Oct 11, 2022

That's nice, but sorry I speak about jobs ;)

Sorry I misread.
I verified through the web client with https://github.com/acsone/queue/blob/6caca1aa057bc38706af3f92c33ee51c5c6c2c7f/queue_job/controllers/main.py#L110 but not sure that's enough.

@okuryan
Copy link

okuryan commented Oct 14, 2022

@pedrobaeza could you, please, publish this repo on odoo app store?

@pedrobaeza
Copy link
Member

That should be @dreispt or @simahawk if I don't remember bad.

@okuryan
Copy link

okuryan commented Oct 15, 2022

@dreispt , @simahawk could you, please, publish queue_job to odoo app store?

@sbidoul sbidoul deleted the 16.0-mig-queue_job-bai branch October 15, 2022 13:14
@sbidoul
Copy link
Member

sbidoul commented Oct 15, 2022

@baimont could you also look for commits in 14.0 that where not in the 15.0 branch? The oca-port tool may help for that.

For instance #443 is missing, it seems.

@baimont
Copy link
Contributor Author

baimont commented Oct 15, 2022

@baimont could you also look for commits in 14.0 that where not in the 15.0 branch? The oca-port tool may help for that.

For instance #443 is missing, it seems.

this pr is merged so i guess this has to be done in a new pr

@dreispt
Copy link
Member

dreispt commented Oct 17, 2022

@okuryan @pedrobaeza This repo is already published in Odoo Apps.
See for example: https://apps.odoo.com/apps/modules/15.0/queue_job/

@pedrobaeza
Copy link
Member

@dreispt you have to publish each branch of each repo...

@okuryan
Copy link

okuryan commented Oct 17, 2022

@dreispt please, let me know when it is published for 16.0 ...

@okuryan
Copy link

okuryan commented Oct 19, 2022

@pedrobaeza are their any ways to reach person responsible for publishing? Or maybe how I can help to speed up publishing process?

@pedrobaeza
Copy link
Member

Sorry, I can't say, as I'm not on OCA board, and don't know other OCA apps store managers.

@dreispt
Copy link
Member

dreispt commented Oct 19, 2022

I tried again and we can now publish 16.0 branches,
It was done for the Queue repo, and this module is now available:
https://apps.odoo.com/apps/modules/16.0/queue_job/

@okuryan
Copy link

okuryan commented Oct 19, 2022

@dreispt yes, I had to write to Odoo support for this and push them through our account manager. They had bug on Odoo app store not allowing to publish Odoo 16

@baimont
Copy link
Contributor Author

baimont commented Oct 19, 2022

@baimont could you also look for commits in 14.0 that where not in the 15.0 branch? The oca-port tool may help for that.

For instance #443 is missing, it seems.

I did a fw port of #443 here : #470
And then I saw this : #453

@sbidoul
Copy link
Member

sbidoul commented Oct 20, 2022

@baimont I think I meant #433, actually. Sorry for the confusion.

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.

8 participants