-
-
Notifications
You must be signed in to change notification settings - Fork 460
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
Migrate v11 queue job subscribe #31
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@guewen I only reviewed files in these commits https://github.com/OCA/queue/pull/31/files/85af69bf6cc064243aff0756848d28b2116297d1..6eac2b205d4db38afb8551d8306a8ee3ab9e0897 .
In terms of the port it looks fine, obviously its a simple one, although I noticed in res_users that we have Jobs Notifications as the string. To be honest I don't know if its right or not, but looks wrong given the help string refers to Job Notifications.
6eac2b2
to
153ab8f
Compare
Changes Unknown when pulling 153ab8f on guewen:migrate-v11-queue_job_subscribe into ** on OCA:11.0**. |
@gdgellatly Thanks! I added a fixup commit, changing the string to "Job Notifications". I don't know either which was correct but at least it's consistent. |
Changes Unknown when pulling 12d1328 on guewen:migrate-v11-queue_job_subscribe into ** on OCA:11.0**. |
12d1328
to
aebcdaf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
runbot build re-triggered
Depends of #30