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

Format files and fix issues found by quality check #211

Merged
merged 6 commits into from
Feb 16, 2024

Conversation

kfahn22
Copy link
Contributor

@kfahn22 kfahn22 commented Feb 14, 2024

Only change is replacing single quotes with ""

Replace single quotes with ""
Copy link
Collaborator

@MKhalusova MKhalusova left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but there may be other potential style issues, have you checked the rest of the file?

@kfahn22
Copy link
Contributor Author

kfahn22 commented Feb 14, 2024

@MKhalusova I would not be surprised about that. How do I check locally? I have used npm to run tests on json files, but I am not sure how to check the mdx files. Is there a reference I could look at?

@MKhalusova
Copy link
Collaborator

Run python utils/code_formatter.py locally (without the --check_only flag), it should fix all the issues

@kfahn22
Copy link
Contributor Author

kfahn22 commented Feb 14, 2024

Thanks! I was trying to pass the computer-vision-course folder as an argument.

@kfahn22
Copy link
Contributor Author

kfahn22 commented Feb 14, 2024

@MKhalusova I have added the fixes for the first two mdx files that were failed the checks. Feature-matchng.mdx is still failing with the change to "".

@kfahn22 kfahn22 mentioned this pull request Feb 14, 2024
@kfahn22
Copy link
Contributor Author

kfahn22 commented Feb 14, 2024

@MKhalusova Yea, It passed the quality checks!

@MKhalusova
Copy link
Collaborator

Hooray! 🎉

@kfahn22 kfahn22 changed the title Update feature-matching.mdx Format files and fix issues found by quality check Feb 15, 2024
Copy link
Collaborator

@lunarflu lunarflu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @kfahn22 ! 🤗

@lunarflu
Copy link
Collaborator

just one thing here I think
Error: Process completed with exit code 1.

@MKhalusova
Copy link
Collaborator

just one thing here I think
Error: Process completed with exit code 1.

If you're referring to the build_pr_documentation job, I don't think it's working yet.

Copy link
Collaborator

@merveenoyan merveenoyan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@merveenoyan merveenoyan merged commit 83f66fc into johko:main Feb 16, 2024
1 of 2 checks passed
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.

4 participants