Clean up nitclk docs #1043
Clean up nitclk docs #1043
Conversation
Bug fixed
'\' in comments now need to be escaped
…efined on Python2
Codecov Report
@@ Coverage Diff @@
## master #1043 +/- ##
=========================================
+ Coverage 89.78% 89.98% +0.2%
=========================================
Files 20 20
Lines 3621 3674 +53
=========================================
+ Hits 3251 3306 +55
+ Misses 370 368 -2
Continue to review full report at Codecov.
|
| @@ -352,7 +352,10 @@ def _replace_attribute_python_name(a_match): | |||
| aname = attr['name'].lower() | |||
|
|
|||
| if config['make_link']: | |||
| return ':py:data:`{0}.Session.{1}`'.format(config['module_name'], aname) | |||
| if config['module_name'] == 'nitclk': | |||
There was a problem hiding this comment.
I'm pretty uncomfortable adding conditional logic based on specific module names to the code-generator logic.
We should really try to keep this generic.
There was a problem hiding this comment.
How? What can we key on instead?
There was a problem hiding this comment.
If driver config had:
- Name of the main class
- Add properties to main class
- Add functions as methods to main class
Could we then make better reuse of the templates and get rid of this conditional logic?
There was a problem hiding this comment.
Yes, but to quote a top notch engineer from a different review "The medicine is worse than the disease".
It would remove using 'nitclk' in the conditionals here (but not remove them), and session.py.mako would become much more complicated and less easy to follow with the conditionals there surrounding large blocks of code.
And I don't think it would be enough to combine the nitclk session.py.mako with the generic one. Or if it is, it would become even more complicated.
There was a problem hiding this comment.
Feels very wrong and hacky, but I can see it being the pragmatic solution. At least we tried.
texasaggie97-zz
dismissed
marcoskirsch’s stale review
Comments addressed pending question
I've updated CHANGELOG.md if applicable.I've added tests applicable for this pull requestWhat does this Pull Request accomplish?
SessionReferenceand the functionsList issues fixed by this Pull Request below, if any.What testing has been done?