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

runtests.py: treat strings within quotes as a single string #316

Closed
wants to merge 1 commit into from

Conversation

xupengfe
Copy link
Contributor

@xupengfe xupengfe commented Jul 4, 2024

Treat strings within quotes as a single string:
It could solve the avx512vbmi/tests like "0 ff b" could not be executed by runtest.py.

Treat strings within quotes as a single string:
It could solve the avx512vbmi/tests like "0 ff b" could not
be executed by runtest.py.

Signed-off-by: Pengfei Xu <[email protected]>
@ysun
Copy link
Contributor

ysun commented Jul 4, 2024

Hi @xupengfe , appreciate your quick response. I have another implement for this issue, that is more "python style"
please review: #317
Add @haoliang-Zhu for more comments, too.

@xupengfe
Copy link
Contributor Author

xupengfe commented Jul 4, 2024

Hi @ysun
I was the first to fix this problem, and according to community guidelines, the maintainer can suggest improvements, but the original fix should be acknowledged.

Yes, the cmd line should be parsed together, it's better.
Additionally, your submitted patch does not account for exception cases.

I included you as 'Co-developed-by' and Zhu Haoliang as 'Reported-by'. Would you agree with this approach?
Please check the #318

Thanks!

@xupengfe
Copy link
Contributor Author

xupengfe commented Jul 4, 2024

Hi @ysun
I was the first to fix this problem, and according to community guidelines, the maintainer can suggest improvements, but the original fix should be acknowledged.

Yes, the cmd line should be parsed together is better.
Additionally, your submitted patch does not account for exception cases. I improved it.
I included you as 'Co-developed-by' and Zhu Haoliang as 'Reported-by'. Would you agree with this approach?
Please check the #318

Thanks!

@ysun
Copy link
Contributor

ysun commented Jul 4, 2024

Hi @ysun , I was the first to fix this problem, and according to community guidelines, the maintainer can suggest improvements, but the original fix should be acknowledged.

Yes, the cmd line should be parsed together is better. Additionally, your submitted patch does not account for exception cases. I improved it. I included you as 'Co-developed-by' and Zhu Haoliang as 'Reported-by'. Would you agree with this approach? Please check the #318

Thanks!

Sure, thanks Pengfei's professional commonts, and the new commit. The approach is good to me.
Please close this PR, let's review the #318

@ysun ysun closed this Jul 5, 2024
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