Fix to HTML Coverage#326
Conversation
1 similar comment
jgebal
left a comment
jgebal
left a comment
There was a problem hiding this comment.
Thanks @lwasylow for your first contribution.
Nice catch with that bug and a really quick fix.
Minor comments on the fix itself.
It would be great if we could have a test created that would be proving the bug existence and also that the fix is working correctly.
This way we avoid regression in future, when changes will be done to this code base.
| if self.coverage_options is not null then | ||
| l_run_schemes := get_run_schemes(); | ||
| l_run_schemes := get_run_schemes(); | ||
| l_run_schemes := coalesce(coverage_options.schema_names, l_run_schemes); |
There was a problem hiding this comment.
It seems that this fix will work well but the variable l_run_schemes changes it's meaning.
Before that it was holding list of schema names that are holding unit tests to run.
With this operation, it will hold the schema names that contain code to gather coverage on.
It actually seems a right thing to do but I would change the variable name.
It should be called l_coverage_schema_names or l_schema_names_to_cover or something like this.
This way we avoid confusion and ambiguity.
Could you also make those two lines into one with single assignment and make sure indents are consistent?
2 similar comments
1 similar comment
2 similar comments
4 participants
Adding ability to overwrite default schemas list used in html code coverage.