mrtazz / checkmake

experimental linter/analyzer for Makefiles
MIT License
1.02k stars 44 forks source link

support checkmake.ini overriding of minphony required targets #88

Closed mcandre closed 1 month ago

mcandre commented 1 year ago

Fixed the minphony implementation to actually support overriding from checkmake.ini file.

Long term, there is some refactoring to do in the data models and overriding logic, such as for maxbodylength and other rules. Specifically, Go tags in the models should do 90% of the work for us. And the INI library we're using already supports this:

https://ini.unknwon.io/docs/advanced/map_and_reflect

Note: The INI library appears to split by comma-with-space (unknown sensitivity to lack of space), while this patch splits by a single space delimiter.

Any special parsing/rendering requirements on top of that automatic marshaling, can be done by overriding INI marshaling methods on the data models.

But refactoring's for another day. I just want the app to work.

This fix would close #86.

If the fix is acceptable, then I would love to see this included in a new release version soon, for the benefit of us DevOps who prefer to use non-HEAD versions of our tools :)

mrtazz commented 1 year ago

thanks for fixing this! Would you be up for also adding a test case that passes the config into the .Run() method and makes sure it gets properly used? Unfortunately we currently only test through configuring required targets on struct instantiation and not passing it to the method. It would be nice to more easily catch things where we don't properly take config as a run parameter into account.