Skip to content
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

[11.0][FIX] web_timeline: add_events call and add jsdocs #1087

Merged
merged 2 commits into from
Oct 26, 2018

Conversation

tarteo
Copy link
Member

@tarteo tarteo commented Oct 19, 2018

Issue: #1077

@simahawk
Copy link
Contributor

@tarteo any chance to have separated commits? Makes really hard to understand where the fix is... 😉

@simahawk simahawk added this to the 11.0 milestone Oct 19, 2018
@simahawk simahawk changed the title [11.0][FIX] add_events calling and add jsdocs [11.0][FIX] web_timeline: add_events call and add jsdocs Oct 19, 2018
@tarteo
Copy link
Member Author

tarteo commented Oct 19, 2018

@simahawk Sorry 😄, Now it should be clear.

Copy link
Contributor

@simahawk simahawk left a comment

Choose a reason for hiding this comment

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

wouldn't be better to delegate events binding to

events: _.extend({}, AbstractRenderer.prototype.events, {
?

@tarteo
Copy link
Member Author

tarteo commented Oct 19, 2018

@simahawk This PR just fixes #1077 and makes travis green (by adding documentation), I'm not trying to refactor it.

@simahawk
Copy link
Contributor

@tarteo I'm not proposing a refactoring: I'm proposing to fix this in a better way 😉
Then you can tell me "is not going to work in this case" and I surrender 😄

@tarteo
Copy link
Member Author

tarteo commented Oct 19, 2018

@simahawk Oh I see, I understand now. Thanks I didn't notice but of course I'll fix it the Odoo way! 👍 👍

Copy link
Contributor

@simahawk simahawk left a comment

Choose a reason for hiding this comment

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

LG :)
Can you squash the last 2 commits?

@simahawk
Copy link
Contributor

plus, please, prefix all commits w/ module name ;)

@tarteo tarteo force-pushed the 11-fix-add-events branch 2 times, most recently from c796669 to 2ad08b5 Compare October 19, 2018 14:08
[ADD] web_timeline: JSdocs
[FIX] web_timeline: calling of add_events
@astirpe
Copy link
Member

astirpe commented Oct 26, 2018

This one can be merged IMO

@pedrobaeza pedrobaeza merged commit 5ea0764 into OCA:11.0 Oct 26, 2018
@pedrobaeza
Copy link
Member

Please include it in 12.0 migration

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.

4 participants