You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
As I mentioned in #1406 I have been working on improving our todo list functionality so that the TodoCommand recognises if there is a configuration file present (either the default one or whatever is passed in via -c option) and if there is, append to that instead of creating a new file.
Unfortunately I hit a snag recently: I thought I could just add another detectors block to our configuration file like this:
---detectors:
UncommunicativeMethodName:
exclude:
- Smelly#x# Auto generated by Reeks --todo flagdetectors:
UncommunicativeVariableName:
exclude:
- Smelly#x
but this doesn't work because ConfigurationFileFinder.load_from_file just discards all non-unique keys.
I see 3 options to fix this:
1.) Find the current detectors block and append the todo list configuration there. This might become very messy very fast, since we'd have take into account all different configuration file layouts there could be. Looking for markers like "last 4 characters indented line in the first detectors block" sounds already incredibly hacky and error-prone.
2.) Read in the existing configuration, append the new todo list configuration and then write it out again. This might kill the formatting of the original file and delete comments. Both not acceptable obviously.
3.) Add the feature to Reek that allows to include additional configuration file via include directive.
What I'd like to suggest:
1.) Update the todo list generation so that it will only update the .reek.yml based on no existing configuration (which corresponds to what we have already since the current TodoList command does not take into account any pre-existing configuration). The task will just abort with a corresponding message if there already is a .reek.yml present. So super simple and just suited for the use case where you introduce Reek into a legacy project. This will also make our todo list feature extremely simple to understand and explain and remove the confusion for now.
2.) Add the feature to Reek that allows to include additional configuration files via include directive and reverse-merge whatever it finds in those files.
3.) Come back to the initial idea of improving the todo list generation and basically continue to work on what I start in the PR #1408 which would make the todo list feature very powerful since you could not just use it for introducing it to legacy projects but also to continuously work on bigger projects where you might not have control over people fixing warnings and you need to get back to a clean slate to continue.
WDYT?
The text was updated successfully, but these errors were encountered:
As I mentioned in #1406 I have been working on improving our todo list functionality so that the TodoCommand recognises if there is a configuration file present (either the default one or whatever is passed in via -c option) and if there is, append to that instead of creating a new file.
Unfortunately I hit a snag recently: I thought I could just add another
detectors
block to our configuration file like this:but this doesn't work because
ConfigurationFileFinder.load_from_file
just discards all non-unique keys.I see 3 options to fix this:
1.) Find the current
detectors
block and append the todo list configuration there. This might become very messy very fast, since we'd have take into account all different configuration file layouts there could be. Looking for markers like "last 4 characters indented line in the first detectors block" sounds already incredibly hacky and error-prone.2.) Read in the existing configuration, append the new todo list configuration and then write it out again. This might kill the formatting of the original file and delete comments. Both not acceptable obviously.
3.) Add the feature to Reek that allows to include additional configuration file via
include
directive.What I'd like to suggest:
1.) Update the todo list generation so that it will only update the .reek.yml based on no existing configuration (which corresponds to what we have already since the current TodoList command does not take into account any pre-existing configuration). The task will just abort with a corresponding message if there already is a .reek.yml present. So super simple and just suited for the use case where you introduce Reek into a legacy project. This will also make our todo list feature extremely simple to understand and explain and remove the confusion for now.
2.) Add the feature to Reek that allows to include additional configuration files via
include
directive and reverse-merge whatever it finds in those files.3.) Come back to the initial idea of improving the todo list generation and basically continue to work on what I start in the PR #1408 which would make the todo list feature very powerful since you could not just use it for introducing it to legacy projects but also to continuously work on bigger projects where you might not have control over people fixing warnings and you need to get back to a clean slate to continue.
WDYT?
The text was updated successfully, but these errors were encountered: