Skip to content
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

src: Deduplicate read enable generation. #108

Closed

Conversation

Blebowski
Copy link

No description provided.

@Blebowski Blebowski marked this pull request as draft May 4, 2024 19:51
@amykyta3
Copy link
Member

amykyta3 commented May 5, 2024

I cannot accept this pull request. I attempted to run the unit tests on this change, and nearly all of them fail.
Of the 150+ tests, only 5 passed. Most did not even compile correctly.

This implementation does not correctly handle any of the required scoping semantics.

@amykyta3 amykyta3 closed this May 5, 2024
@Blebowski
Copy link
Author

Yup, it was only draft. I need to bring-up the questa free edition to get the tests running.

@amykyta3
Copy link
Member

amykyta3 commented May 5, 2024

Again, as discussed in #103, I would really want to see evidence that this improves performance. Otherwise I do not have a compelling reason to accept PRs for issue #103 that add unnecessary complexity to the tool.

@Blebowski
Copy link
Author

Understood.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants