Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
amomchilov
changed the title
amomchilov
force-pushed
the
Alex/tidy-sorbet-config
branch
from
d8279da to
487fe1b
Compare
|
|
||
| #: bool | ||
| attr_accessor :no_stdlib | ||
| attr_reader :no_stdlib |
There was a problem hiding this comment.
So we can add new paths/ignore/allowed_extensions but not change this flag?
Should we freeze the arrays then? We could also type them as Enumerable[String].
There are places where we do expect the config to not be read only: https://github.com/Shopify/spoom/blob/main/lib/spoom/coverage.rb#L19
There was a problem hiding this comment.
So we can add new paths/ignore/allowed_extensions but not change this flag?
Yeah basically.
Should we freeze the arrays then?
Not without changing the places like the one you mentioned, or that I change in this PR.
This is very much a half-measure, but still a marginal improvement
There was a problem hiding this comment.
What are the benefits of making it half-read-only? Can we keep it writable?
amomchilov
force-pushed
the
Alex/tidy-sorbet-config
branch
from
487fe1b to
47e43de
Compare
|
Are these 2 PRs performance optimization? Or do they fix some bad usage patterns we saw somewhere? |
3 participants
Well kind of, there's still some places where the arrays are edited in-place.