-
Notifications
You must be signed in to change notification settings - Fork 24.4k
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
[CI] Test failure: D9614432 adds thecount dependency, but it's missing from open source #20994
Comments
cc @gengjiawen who discovered this in b5d908b#diff-c348396d3d79622651a08ef206c42be5. I've pinged the author of that commit to see what can be done - it's unclear to me yet if thecount can be open sourced. |
Maybe the open source version could use a simple no-op shim? |
@empyrical I think that's what we should go with. This thecount library is not meant to be open source. |
Any response from the author ? |
Yeah, I already chatted with the author. The code referenced here is not meant to be open sourced. We need to either roll back, or add a no-op shim as @empyrical suggested. We also need to toughen up our internal land blocking tests to ensure they can catch issues like these in the future. |
If the author can't fix it in short time, I am inclined to revert the commit, blocking master ci for a long time is not a good idea. Also, for internal test, is it possible fb use something like docker to avoid some private code ? |
No, we cannot use Docker. |
We're reverting the change. |
This can be observed in Circle CI with the following failure:
The text was updated successfully, but these errors were encountered: