-
Notifications
You must be signed in to change notification settings - Fork 22
Module readme tab improvements #295
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
…atting and changed gradle code
|
||
try { | ||
var moduleBean = getParentHub().getModuleRegistry().findModuleClass(beanItem.getBean().moduleClass); | ||
configTabs.addTab(new ReadmePanel(moduleBean), "README"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought you could use the config class itself instead of looking for the module class. Did you do this for a specific reason?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some drivers have their config in a subdirectory, and getting the resource from the config class loader would require that subdirectory also exist in resources and hold the README. Since some drivers have multiple classes named some variation of "Config" and multiple subdirectories, I figured it would be a bit messy to try and figure out where to put the README from inside the build.gradle script.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suppose the same issue could happen with the module class though.
The key is that the README needs to be in the same package as the class we use to load it. If we pick the config, the README has to be in the same package as the config class. If we pick the module, the README has to be in the same package as the module class.
And actually a module can even contain several drivers so there could be several READMEs but as long as the drivers/config classes are in different packages (and so the README as well), then it would work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right now I'm using the node's gradle build script to copy the readme from the root dir of the module into the resources directory with the path based on the activator's package. I didn't realize that there could be multiple drivers in the same module. Should I switch it back to using the config class loader for finding the resource?
I'm not too familiar with gradle, so it might take me some time to figure out. Not sure how possible/valuable it would be to automate this at that point.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, I see what you mean. This is tricky...
If there are several drivers in the same module then the READMEs would have to be different from the main module README anyway so I guess in that case we would place them in the right package manually and there is no need for the Gradle automation.
If there is only one README then I would recommend putting it in the same package as the config class, but it's harder to figure it out automatically.
One way to solve it is to add the Gradle automation to the module itself rather than a generic one for the entire node. This way the correct package can be set in the script. This way it would work on any node deployment too and it becomes a responsibility of the module developer to add that, which I think is ok.
generates internal errors. Caller should handle null instead.
generates internal errors. Caller should handle null instead.
background write thread (leading to a complete DB lock).
* Added tab for readme in the module panel * Removed DefaultREADME.md, added default to Readme.java * Moved readme tab, minor improvements and closeable instructions
Commits got really messy here and I think some changes were lost. I'm going to make a new PR from a new branch. |
Changed directory for readme discovery, improved default html formatting, changed instructions and sample gradle code.