Removed over-nesting in resources file and added test #498
Conversation
There was a problem hiding this comment.
@lrjball
Thanks for this PR I think it's good that you added a unit test to ensure that deep path will fail tests!
I think it will be more explicit than raising error in the autogen and I +1 that the path must not be to deep and actual limitation is perfect.
There's already instruction in the CONTRIBUTING.md file, but when I added openstack I stick too much to the official structure and didn't test enough.
I hope it will be merge soon as some of the icons for openstack are not usable now.
If I can ask you for one change, can you update the test comment and use the
"resources///"
instead of
"resources/provider/module/icon.png"
To be consistent with the terms used in the CONTRIBUTING.md file.
updated doc string of test function
|
Thanks @gabriel-tessier That makes sense, I've made that change now |
* add_raspbian_gen_os: update raspbian.png for size Added raspbian.png for Raspberry Pis. Add Raspberry Pi logo for describing RPis. fix: removed over-nesting in resources file and added test (mingrammer#498) feat(node): add Onprem puppet (mingrammer#492) feat(node): add SVN (mingrammer#426) feat(node): add Dynatrace in monitoring category (mingrammer#471) docs(contributing): add black tool requirement for autogen.sh (mingrammer#434) feat(node): add Tyk node (mingrammer#449) feat(node): add starlette framework (mingrammer#443) chore(deps-dev): bump docusaurus from 1.14.6 to 1.14.7 in /website (mingrammer#485) chore(deps): bump jinja2 from 2.11.3 to 3.0.1 (mingrammer#534) fix(website): add IBM in the side menu (mingrammer#574) chore(deps): bump graphviz from 0.16 to 0.17 (mingrammer#561) docs: replace master/slave terminology in Cluster example (mingrammer#521) fix(autogen): run autogen after IBM provider added. (mingrammer#569) docs(all): update from main to primary (mingrammer#573) feat(node): latex logo (mingrammer#565) feat(node): add k3s icon (mingrammer#575)
Fixes #497
To fix the issue of some openstack icons (20 in total) not being rendered properly, I have removed some of the extra nesting and brought all of the module directories in openstack under the openstack/ dir instead of being nested. I've also added a test to make sure extra nesting isn't included in the future.
I thought this was the easiest fix. The code could be rewritten to allow arbitrary nesting in the resources dir, but that may be overkill and it is probably best that the import paths aren't too long anyway.