Excluded modules should not be added to task graph #167
Conversation
Codecov Report
@@ Coverage Diff @@
## main #167 +/- ##
============================================
+ Coverage 50.37% 50.46% +0.09%
- Complexity 66 67 +1
============================================
Files 13 13
Lines 532 535 +3
Branches 99 99
============================================
+ Hits 268 270 +2
- Misses 237 238 +1
Partials 27 27
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
| project.pluginManager.withPlugin(pluginId) { | ||
| getAffectedPath(testType, project)?.let { path -> | ||
| if (AffectedModuleDetector.isProjectProvided(project)) { | ||
| if (AffectedModuleDetector.isProjectProvided(project) && !isExcludedModule(config, path)) { |
There was a problem hiding this comment.
we can call return before here. Calling afterEvaluate and withPlugin is not necessary in my mind.
| value = rootProject.extensions.findByName(AffectedModuleConfiguration.name), | ||
| lazyMessage = { "Unable to find ${AffectedTestConfiguration.name} in $rootProject" } | ||
| ) as AffectedModuleConfiguration | ||
| val mainConfiguration = requireConfiguration(rootProject) |
There was a problem hiding this comment.
this method calls quite often in big multymodule projects.
- too much boilerplate
- DRY
- we can do small optimization with singleton
There was a problem hiding this comment.
After the changes to task registration this won't be called as often so that isn't a huge deal
- Can you explain what you mean by boilerplate? I don't see anything here out of the ordinary
- I understand what DRY is, this infact removes duplicate code and fixes a but committed by you (wrong configuration.name)
- Extensions is a map meaning looking up by name is a constant time function call, essentially the same as doing it via a singleton. Plus we don't have to worry about the lifecycle of setting the singleton
There was a problem hiding this comment.
I recognized your comments in my previous PR. Actually, I was wrong. Thank u for the explanation.
I closed my previous PR, a bit pity my time. But it is the right way.
We can merge your version for solving the problem.
Fixes #159
I believe this fix should handle excluded modules not being added at all to the task graph by checking the configuration before adding it