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

Updated few changes to the code and updated the files which had few typos in it. #33638

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

vignesh1507
Copy link

I added few lines of code in parse_test_output.py as it had few issues, i made sure not to make changes to the original file that's why i made a new file for the updated code. I have also updated few other files which had typos in it.

I have updated the original parse_tesst_outputs.py file by adding  file lines of code in it and made a new file for it. Hope this resolves the issue.
@ydshieh
Copy link
Collaborator

ydshieh commented Sep 27, 2024

Thanks! But

i made sure not to make changes to the original file that's why i made a new file for the updated code

why not directly change the file? A new file like this is difficult to review as we don't know what exactly are the issues and how the fixes are relevant.

@vignesh1507
Copy link
Author

Ok then, i'll add my changes to the original file.

Deleting this file and updating the changes back to the original file.
@ydshieh
Copy link
Collaborator

ydshieh commented Sep 27, 2024

Thanks for updating! A few comments around the changes to explain what the issues are and explain a bit how the fix is done is also essential 🙏

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