Keep "test types" info in one location #28759
Conversation
This commit aims to centralize the "test types" constant used by `CodeStatistics`. Previously, if one wanted to add a test type, you would have to update two different constants. Now, you only have to update one, and the changes will be properly propogated. Sidenote: `CodeStatistics` is a nodoc'ed class, so external users should not be using it. Inspired by rails#28744.
| @@ -1,7 +1,18 @@ | |||
| # While global constants are bad, many 3rd party tools depend on this one (e.g | |||
| # rspec-rails & cucumber-rails). So a deprecation warning is needed if we want | |||
| # to remove it. | |||
There was a problem hiding this comment.
☝🏻 I imagine this applies to TEST_TYPES too.
Not to mention the disruption that pulling out half its values could create... or the awfulness of introducing another global constant.
There was a problem hiding this comment.
Technically TEST_TYPES is nested under CodeStatistics which is a nodoc class... so we explicitly say that this is internal. We're also just trading one constant for another, with the new one being more useful :)
| @@ -10,20 +21,13 @@ STATS_DIRECTORIES = [ | |||
| %w(Channels app/channels), | |||
| %w(JavaScripts app/assets/javascripts), | |||
| %w(Libraries lib/), | |||
| %w(APIs app/apis), | |||
There was a problem hiding this comment.
It's still there, I think the diff is just pointing out that I removed the comma :)
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
Summary
This commit aims to centralize the "test types" constant used by
CodeStatistics. Previously, if one wanted to add a test type, youwould have to update two different constants. Now, you only have to
update one, and the changes will be properly propogated.
Other Information
Sidenote:
CodeStatisticsis a nodoc'ed class, so external users shouldnot be using it.
Inspired by #28744.