-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
[16.0][MIG] web_widget_bokeh_chart: Migration to 16.0 #2526
[16.0][MIG] web_widget_bokeh_chart: Migration to 16.0 #2526
Conversation
467e3c3
to
e173337
Compare
Avoid "Duplicate explicit target name" error using anonymous hyperlink references Ref.: https://stackoverflow.com/questions/5464627/how-to-have-same-text-in-two-links-with-restructured-text/14067756#14067756
…ontainer element.
…mit dynamic updates
Includes some manual fixes to silent ESLint warnings.
improve the documentation of web_widget_bokeh_chart by providing the required imports to be put in the Python code and fixing the code to so that it works on the version 15 of the module.
aafb122
to
8b41b6d
Compare
8b41b6d
to
4639a13
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.
Functional test ok. Thanks for the work.
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.
LGTM
I think I can merge as maintainer of the module, let's see... /ocabot merge nobump |
Sorry @LoisRForgeFlow you are not allowed to merge. To do so you must either have push permissions on the repository, or be a declared maintainer of all modified addons. If you wish to adopt an addon and become it's maintainer, open a pull request to add your GitHub login to the |
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.
LGTM!
@OCA/web-maintainers I guess, the module maintainer merging rights does not work on migrations. Could someone do the honors? Thanks! |
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.
I think I can merge as maintainer of the module, let's see...
Thanks for porting this usefull module.
Quick question : why do you pin the version bokeh ? AFAIK, the lib is stable, so it could be great to install allways the latest supported version. don't you think or do you see a risk ?
otherwise LGTM.
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.
@legalsylvain because the pip package need to match the JS assets being added with this module
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.
oh ! you're right. Thanks.
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.
/ocabot merge patch
What a great day to merge this nice PR. Let's do it! |
Congratulations, your PR was merged at 9421ebe. Thanks a lot for contributing to OCA. ❤️ |
No description provided.