-
Notifications
You must be signed in to change notification settings - Fork 6.6k
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
add easy diff for document tracking #7498
add easy diff for document tracking #7498
Conversation
309b2e9
to
90427cf
Compare
This script is based on the assumption that documents in other languages are fully synchronized with the en document at a commit. |
90427cf
to
d2704d9
Compare
docs/tools/easy_diff.py
Outdated
CLICK_HOUSE_REPO_HOME = os.path.join(os.path.dirname(SCRIPT_PATH), '../../') | ||
|
||
|
||
def diffFile(reference_file, working_file, git, temp_diff): |
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.
def diffFile(reference_file, working_file, git, temp_diff): | |
def diff_file(reference_file, working_file, git, temp_diff): |
snake_case style is more common in python for function names and as far as I remember nearby scripts use it, let's change this? (here and below)
docs/tools/easy_diff.py
Outdated
from tempfile import NamedTemporaryFile | ||
|
||
SCRIPT_PATH = os.path.abspath(__file__) | ||
CLICK_HOUSE_REPO_HOME = os.path.join(os.path.dirname(SCRIPT_PATH), '../../') |
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.
not so important, but I think:
- it's better to write
CLICKHOUSE
in one word os.path.join(os.path.dirname(SCRIPT_PATH), '..', '..')
looks better
working_item = os.path.join(working_directory, list_item) | ||
reference_item = os.path.join(reference_directory, list_item) | ||
if diffFile(reference_item, working_item, git, temp_diff) if os.path.isfile(reference_item) else diffDirectory(reference_item, working_item, git, temp_diff) != 0: | ||
return 1 |
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.
shouldn't we use this number as exit code below where we call these functions?
docs/tools/easy_diff.py
Outdated
if __name__ == '__main__': | ||
git = cmd.Git(CLICK_HOUSE_REPO_HOME) | ||
git_pager = git.execute(['git', 'var', 'GIT_PAGER']) | ||
working_language = os.path.join(CLICK_HOUSE_REPO_HOME, 'docs', sys.argv[1]) |
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'd better start using argparse
module right away to make it more flexible and extendable. For example there could be an option to skip the git pager part.
@zhang2014 it's better to add this comment to |
d2704d9
to
f8d0ebc
Compare
f8d0ebc
to
6db3da8
Compare
f8e50d6
to
281c81c
Compare
I hereby agree to the terms of the CLA available at: https://yandex.ru/legal/cla/?lang=en
Category :
Detailed description (optional):
for example:
use stdout print :
use git pager print: