Added/Fixed Typing for all files in vunit.ui package#961
Added/Fixed Typing for all files in vunit.ui package#961g0t00 wants to merge 9 commits intoVUnit:masterfrom
Conversation
vunit/ui/packagefacade.py
Outdated
| from vunit.design_unit import DesignUnit | ||
|
|
There was a problem hiding this comment.
| from vunit.design_unit import DesignUnit | |
| from vunit.design_unit import DesignUnit |
Not sure if any linting rules are applied, but this seems to make more sense either way.
There was a problem hiding this comment.
I do not really catch what your saying. But I did not find clear style re: import statements.
Also after doing all the changes I realized that vunit uses relative imports for most/all internal imports.
I personally do not like relative imports in python, but for consistencies sake I would change. If somebody from the core team can pitch in...
There was a problem hiding this comment.
Sorry for being unclear, the point was that if there is an empty line, it should probably not happen between the vunit imports.
Now
from typing...
from vunit...
from vunit...
Better
from typing...
from vunit...
from vunit...
(Tools like isort would probably have done something like the latter anyway.)
There was a problem hiding this comment.
Ah yes, thank you. Totally agreed.
(normally I just let isort handle imports)
|
Typing always helps! Some minor comments (I have no powers to approve or merge or whatever, but for what it is worth). |
|
After rebasing on top of branch Lintinghttps://github.com/VUnit/vunit/actions/runs/8236613480/job/22523448088?pr=961#step:5:84 Building sdisthttps://github.com/VUnit/vunit/actions/runs/8236613480/job/22523449340?pr=961#step:5:43 Furthermore:
Ref #601. |
No description provided.