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 mig dbfilter from header #1182

Merged
merged 18 commits into from
Apr 4, 2018

Conversation

yelizariev
Copy link
Member

PR with updates from #1137

@pedrobaeza pedrobaeza added this to the 11.0 milestone Mar 8, 2018
@pedrobaeza pedrobaeza mentioned this pull request Mar 8, 2018
92 tasks
updates were made in OCA#1137 by @TimLai125

and small lint fixes by @yelizariev
@yelizariev yelizariev force-pushed the 11.0-mig-dbfilter_from_header branch from 47eb3a4 to dc50d04 Compare March 12, 2018 17:13
@jaredkipe
Copy link
Contributor

LGTM

Copy link

@ilmir-k ilmir-k left a comment

Choose a reason for hiding this comment

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

I've just checked this ported module on my test server (with nginx), it works correctly 👍

@ilmir-k
Copy link

ilmir-k commented Apr 3, 2018

@pedrobaeza could you review it please? There are not many changes

@pedrobaeza
Copy link
Member

Sorry, I'm not a user of this module, so I can't say if it's correctly migrated.

@jaredkipe
Copy link
Contributor

Can we merge this?

@pedrobaeza
Copy link
Member

Well, I see that you haven't reviewed properly (putting the approval), so I didn't see it your comment, but as there are 2 reviews counting yours and more than 5 days, I merge.

@pedrobaeza pedrobaeza merged commit 6ed1793 into OCA:11.0 Apr 4, 2018
@jaredkipe
Copy link
Contributor

@pedrobaeza Apologies, I see nothing in my UI that would indicate that I can do anything other than comment on this.
What should I be looking for to properly 'put the approval' on this?

@pedrobaeza
Copy link
Member

Please see the image:

seleccion_005

@pedrobaeza
Copy link
Member

But now you can't approve as it's merged. Next time 😉

Copy link
Contributor

@jaredkipe jaredkipe left a comment

Choose a reason for hiding this comment

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

LGTM

@jaredkipe
Copy link
Contributor

jaredkipe commented Apr 4, 2018

Oh apparently I can! Thanks!
Stange.. twice with one click?

SiesslPhillip pushed a commit to grueneerde/OCA-server-tools that referenced this pull request Nov 20, 2024
Syncing from upstream OCA/server-tools (16.0)
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.