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

[opencv3] Upgrade to version 3.4.8 #8623

Merged
merged 1 commit into from
Nov 4, 2019

Conversation

LilyWangL
Copy link
Contributor

@LilyWangL LilyWangL commented Oct 16, 2019

Upgrade opencv3 to version 3.4.8 and fix old patches.
Related opencv4 PR: #8557

@LilyWangL LilyWangL added the info:internal This PR or Issue was filed by the vcpkg team. label Oct 16, 2019
@LilyWangL LilyWangL marked this pull request as ready for review October 17, 2019 01:41
@JackBoosY JackBoosY self-requested a review October 17, 2019 11:33
@cdcseacave
Copy link
Contributor

pls can anybody explain why this is not merged yet?

@JackBoosY
Copy link
Contributor

@vicroms Ready to merge now.

@ras0219-msft ras0219-msft merged commit df266bd into microsoft:master Nov 4, 2019
@cenit
Copy link
Contributor

cenit commented Nov 5, 2019

I lost track of this PR
May I say one thing: why?
This PR was not ready and is bugged:

opencv3[contrib] is broken, downloading is not disabled anymore, ocv_update is used in a patch and not through commands passed by the portfile... also I suspect that the dlls/lib are treated in the new way and so some new parameters should be passed to the configure step, something similar to what has already been requested since a long time ago in the opencv4 pr cited above but never done.

It looks like I should give up keeping opencv in a proper shape here on vcpkg, because the moment a PR about it is accepted, the moment it is almost surely damaged.

@vicroms sorry for ranting, but to be honest my thoughts now are that I might not contribute anymore to vcpkg, at least not without a proper CI in place to check features better, to preserve port quality and to at least guarantee not to regress going on (something that is continuously happening here)
Of course, this requires a major improvement in revision quality and procedures.

pls can anybody explain why this is not merged yet?

because it was broken, and still was when merged

cbezault added a commit that referenced this pull request Nov 5, 2019
vicroms added a commit that referenced this pull request Nov 5, 2019
sthagen added a commit to sthagen/microsoft-vcpkg that referenced this pull request Nov 5, 2019
Revert "[opencv3] Upgrade to version 3.4.8 (microsoft#8623)" (microsoft#8911)
@vicroms
Copy link
Member

vicroms commented Nov 8, 2019

Hi @cenit,

Thanks for your feedback and contributions to vcpkg – you’re one of our most active community members. I'm sorry about the issues with the OpenCV port and moving forward we’re committed to improve the experience and collaboration with our contributors.

For now, I've reverted the changes, as in it's current state, this PR regresses OpenCV features.

I would like to unblock this update, and as I understand from your comments on this and the related PR (#8557) there are changes that need to be done in the PR and in our CI testing to make sure that no features are regressed.

Moving forward, the vcpkg team would like to start a conversation with you and other OpenCV contributors to better understand how we can expand our CI coverage to ensure a higher quality control for further OpenCV upgrades/changes.

Also, if you have any feedback for other areas of improvement in vcpkg please feel free to reach me (viromer@microsoft.com) or the team at vcpkg@microsoft.com. We would be glad to receive your insights and opinions.

@cdcseacave
Copy link
Contributor

Hi there, can you pls solve this and merge?

@cenit
Copy link
Contributor

cenit commented Jan 15, 2020

i'm very very busy at the moment, sorry, so I cannot guarantee any timing. I can try to push it a little bit up in my stack, but i'd be happier to help reviewing a PR if it happens

@JackBoosY
Copy link
Contributor

@cenit I will do this job.

@cenit
Copy link
Contributor

cenit commented Jan 16, 2020

@JackBoosY thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
info:internal This PR or Issue was filed by the vcpkg team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants