-
Notifications
You must be signed in to change notification settings - Fork 268
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
Fix issues when trying to parameterize Verilog modules #945
base: master
Are you sure you want to change the base?
Conversation
…format to fix issue with Verilog being case-sensitive.
Hi, any chance for having someone look at this PR? |
I think it maybe is better to either:
|
@oscargus I modified it as you suggested. |
Thanks! (I should probably have mentioned that I cannot approve or merge anything, but I think/hope that the suggested changes increases the chance of having it merged...) |
Can you possibly contact someone who is in charge of such approvals? |
I can try to ping in @LarsAsplund and @kraigher . My impression is that they have been quite busy recently though. (There may be other people that have the correct powers as well.) |
Any chance to have this fix merged any time soon? |
More than a year passed - is this repository still maintained somehow? |
@piotrva Sorry for the delay. VUnit has a lot of users, which is good, but unfortunately we don't have maintainer bandwidth to meet that. Working on solutions... Regarding your PR. First of all, all tests must be green for a PR to pass. Not sure why they went red the last time since the logs are deleted when they get too old. I would also recommend a solution that doesn't involve code related to VHDL as this is a Verilog issue. What I recommend is that you don't let Provided the all tests pass after this change, there should also be a test covering this issue. I would recommend looking into tests/acceptance/artificial/verilog/ to see if those tests can be extended to include this casing issue |
Hi, thanks for the response - is there a way the tests can be run again so I can see logs? |
@piotrva You can run all the tests locally, see https://vunit.github.io/contributing.html. However, if you do the update I suggested and push that, the CI will start and run the tests (I may have to approve the run first). After that the logs are visible. Make sure that you rebase your change on the HEAD of VUnit. |
This fixes #944 by trying to execute an original version (case-insensitive for VHDL compatibility) with the fall-back for use with Verilog and non-lower-case module names.