-
Notifications
You must be signed in to change notification settings - Fork 90
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
Make virtualize_frameworks the default #483
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 this is now pretty robust and stabilized and ready for prime. Most of the big builds I've heard about using rules_ios had to enable it because the perf was bad otherwise. Atop from build perf, the IDE is improved with this from multiple angles!
Do you have a strong reason to rip it out: e.g. it's hard to maintain or broken for the other case? Otherwise, I'd suggest to add an opt in / add opt out flag --features apple.incompatible_disable_virtual_frameworks
and give it ~6-12 months to bake. This will have the effect of migrating people and giving ones we break reasonable to fix their issues or doing something else it if they don't / can't want to use it like this.
We can also remove the dup'd over github action shards when it's gone! |
Yeah, this is a better plan. |
There might be some tests to add / use cases to fix before we enable this as default. For example: #500 does not work with VFS enabled. Im not sure if this is a one-off issue or if we'll see more issues like this by flipping it on. |
#500 is now closed so making this the default / cleaning up where the flags are used seems like a reasonable change. |
I'm still hoping to make it default but leave it in - unless it becomes a big maintenance problem for us or other reasons to really gut it! If you've got to another use case e.g. #500 we should try to triage and resolve them. Making it default also might encourage people with issues to fix it but atleast give the escape hatch |
Since this performs better and we keep telling people to enable it, maybe we should make it the default?