Skip to content

[12.0][MIG] web_ir_actions_act_view_reload #1104

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Nov 21, 2018
Merged

[12.0][MIG] web_ir_actions_act_view_reload #1104

merged 3 commits into from
Nov 21, 2018

Conversation

alexey-pelykh
Copy link
Contributor

No description provided.

@alexey-pelykh
Copy link
Contributor Author

@pnajman-modoolar would like to ask for your review, if you don't mind

This was referenced Nov 19, 2018
Closed
Closed
@pedrobaeza pedrobaeza added this to the 12.0 milestone Nov 19, 2018
pnajman-modoolar
pnajman-modoolar approved these changes Nov 19, 2018
View reviewed changes
Copy link
Contributor

@pnajman-modoolar pnajman-modoolar 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 :)
Approved.

@alexey-pelykh
Copy link
Contributor Author

@pnajman-modoolar Thanks for the review! Do you know anyone else to summon who may review as well?

@pnajman-modoolar
Copy link
Contributor

@alexey-pelykh You could try with good people who reviewed version 11.0 of this module.
I would look them up in the 11.0 pull request.

@alexey-pelykh
Copy link
Contributor Author

Good people of 11.0 PR (@elicoidal @simahawk @bealdav @zaoral), I summon you to take a look on this PR if you don't mind

elicoidal
elicoidal approved these changes Nov 20, 2018
View reviewed changes
Copy link

@elicoidal elicoidal left a comment

Choose a reason for hiding this comment

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

LGTM

@alexey-pelykh alexey-pelykh force-pushed the 12.0-mig-web_ir_actions_act_view_reload branch from d18a6d4 to 4f2e04e Compare November 20, 2018 04:50
@alexey-pelykh
Copy link
Contributor Author

Thanks for the reviews! I'd be glad to help likewise, thus feel free to summon me 👍

simahawk
simahawk requested changes Nov 20, 2018
View reviewed changes
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, just a question

@alexey-pelykh alexey-pelykh force-pushed the 12.0-mig-web_ir_actions_act_view_reload branch from 4f2e04e to 152e1f5 Compare November 20, 2018 08:39
@alexey-pelykh
Copy link
Contributor Author

@simahawk please re-review

nikul-serpentcs
nikul-serpentcs suggested changes Nov 20, 2018
View reviewed changes
Copy link
Member

@nikul-serpentcs nikul-serpentcs left a comment •
edited
Loading

Choose a reason for hiding this comment

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

IMO Check Jslint

nikul-serpentcs
nikul-serpentcs approved these changes Nov 20, 2018
View reviewed changes
Copy link
Member

@nikul-serpentcs nikul-serpentcs left a comment

Choose a reason for hiding this comment

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

Otherwise Code LGTM

@alexey-pelykh
Copy link
Contributor Author

@nikul-serpentcs is there an automated check for it? I'm not sure how to run it on Odoo

@nikul-serpentcs
Copy link
Member

@alexey-pelykh Could you please check here

@simahawk
Copy link
Contributor

@alexey-pelykh LG, can you check the warnings @nikul-serpentcs linked?

@alexey-pelykh alexey-pelykh force-pushed the 12.0-mig-web_ir_actions_act_view_reload branch 2 times, most recently from 0271ccb to 8f23675 Compare November 20, 2018 18:44
@alexey-pelykh
Copy link
Contributor Author

@simahawk @nikul-serpentcs all good now

@simahawk
Copy link
Contributor

I still see web_ir_actions_act_view_reload/static/src/js/web_ir_actions_act_view_reload.js:25: [W7903(javascript-lint), ] Trailing spaces not allowed. [Error/no-trailing-spaces]

Am I wrong?

Sorry for this boring requirements but if we don't fix them while we here they are going to stay forever and maybe become errors in the future... thanks for your patience! 😉

@alexey-pelykh alexey-pelykh force-pushed the 12.0-mig-web_ir_actions_act_view_reload branch from 8f23675 to 633e0c8 Compare November 21, 2018 10:14
@alexey-pelykh
Copy link
Contributor Author

@simahawk sorry, dunno where I've looked exactly yet I can swear this warning wasn't there 🙈fixed!

@simahawk simahawk merged commit 7077c35 into OCA:12.0 Nov 21, 2018
@simahawk
Copy link
Contributor

@alexey-pelykh thanks :)

@alexey-pelykh alexey-pelykh deleted the 12.0-mig-web_ir_actions_act_view_reload branch November 21, 2018 11:14
@guewen guewen mentioned this pull request Dec 16, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@simahawk simahawk simahawk approved these changes

@elicoidal elicoidal elicoidal approved these changes

@nikul-serpentcs nikul-serpentcs nikul-serpentcs approved these changes

@pnajman-modoolar pnajman-modoolar pnajman-modoolar approved these changes

Assignees
No one assigned
Labels
approved
Projects
None yet
Milestone
12.0
Development

Successfully merging this pull request may close these issues.
8 participants
@alexey-pelykh @pnajman-modoolar @nikul-serpentcs @simahawk @elicoidal @pedrobaeza @OCA-git-bot @oca-travis