-
Notifications
You must be signed in to change notification settings - Fork 165
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
β¨ Run pre-commit checks and fix issues in GenAI code style #663 #905
base: master
Are you sure you want to change the base?
Conversation
test $(git diff --name-only --diff-filter=A -z $(git hash-object -t tree /dev/null) | LC_ALL=C tr -d '[ -~]\0' | wc -c) == 0 | ||
- name: Check for non-ASCII characters in files | ||
run: "! git grep -n '[^ -~]' -- ':(exclude)SECURITY.md' ':(exclude)LICENSE' ':(exclude)third-party-programs.txt' ':(exclude)tests/python_tests/README.md' ':(exclude)llm_bench/python/README.md' ':(exclude)samples/cpp/beam_search_casual_lm/README.md' ':(exclude)samples/cpp/benchmark_genai/README.md' ':(exclude)samples/cpp/chat_sample/README.md' ':(exclude)samples/cpp/greedy_casual_lm/README.md' ':(exclude)samples/cpp/multinomial_causal_lm/README.md' ':(exclude)samples/cpp/prompt_lookup_decoding_lm/README.md' ':(exclude)samples/cpp/speculative_decoding_lm/README.md' ':(exclude)samples/cpp/stable_diffusion/README.md' ':(exclude)samples/cpp/whisper_speech_recognition/README.md' ':(exclude)samples/python/beam_search_casual_lm/README.md' ':(exclude)samples/python/benchmark_genai/README.md' ':(exclude)samples/python/chat_sample/README.md' ':(exclude)samples/python/greedy_casual_lm/README.md' ':(exclude)samples/python/multinomial_causal_lm/README.md' ':(exclude)samples/python/whisper_speech_recognition/README.md' ':(exclude)src/README.md' ':(exclude)src/docs/BUILD.md' ':(exclude)src/docs/DOCKER.md' ':(exclude)src/docs/SUPPORTED_MODELS.md'" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems you've included all .md
. Can it be (exclude)**/*.md
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need the submoudle
model_id, path, tokenizer, model_opt, pipe = read_model( | ||
(model_descr[0], model_descr[1] / "_test_chat"), add_special_tokens=False | ||
) | ||
>>>>>>> Stashed changes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's why you shouldn't have extended the task with other code style checks. It becomes hard to manage.
Resolve the conflict.
run: | | ||
pip install black flake8 pre-commit | ||
- name: Run Black | ||
run: black --check . |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please, align the code style requirements with the main repo https://github.com/openvinotoolkit/openvino if you are going to keep this. If it's not done so yet.
π οΈ Implemented basic pre-commit checks to improve code quality across the project
π Added checks for:
π§ Fixed detected code style issues and excluded expected failures (e.g., non-ASCII symbols in README.md) to maintain testing integrity.
π This resolves issue #663 by establishing a consistent coding style and enhancing maintainability.