Conversation
|
Can we have some more detail in the commit message please? I am particularly curious as to why this wasn't caught by the unit tests? Can we add one to check correct behaviour? |
Where should these tests run? 🙃 |
|
Nevermind, found it. There is no tests that uses that code. It directly tests with the subfunction. |
|
|
||
| # add ignored components | ||
| components_included.union(ignore_components) | ||
| components_included |= set(ignore_components) |
There was a problem hiding this comment.
For readability I'd prefer:
components_included.update(ignore_components)or:
components_included = components_included.union(ignore_components)There was a problem hiding this comment.
I'm not a big fan of that. It's very verbose.
|
Whoops, we should add at least a |
|
Code is fine (I think, I defer to James). But the documentation (aka commit logs) could be improved :)
Also I think it would be efficient to merge the other 2 commits into 1 with a nice explanation. |
The results of the union operations needs to be stored again.
The code was not working