Skip to content

add easy diff for document tracking#7498

Merged
alexey-milovidov merged 4 commits intoClickHouse:masterfrom
zhang2014:docs/diff_script
Nov 1, 2019
Merged

add easy diff for document tracking#7498
alexey-milovidov merged 4 commits intoClickHouse:masterfrom
zhang2014:docs/diff_script

Conversation

@zhang2014
Copy link
Contributor

@zhang2014 zhang2014 commented Oct 27, 2019 •
edited
Loading

I hereby agree to the terms of the CLA available at: https://yandex.ru/legal/cla/?lang=en

Category :

  • Documentation

Detailed description (optional):

for example:

☁  tools [docs/diff_script] ⚡ ./easy_diff.py zh/data_types

use stdout print :

Need translate document:/Users/coswde/Source/ClickHouse/docs/tools/../../docs/en/data_types/uuid.md

use git pager print:

diff --git a/docs/en/data_types/domains/ipv6.md b/docs/en/data_types/domains/ipv6.md
index 1bfbe3400b..e2abaff017 100644
--- a/docs/en/data_types/domains/ipv6.md
+++ b/docs/en/data_types/domains/ipv6.md
@@ -4,13 +4,13 @@
 
 ### Basic Usage
 
-``` sql
+```sql
 CREATE TABLE hits (url String, from IPv6) ENGINE = MergeTree() ORDER BY url;
 
 DESCRIBE TABLE hits;
 ```
 
-```
+```text
 ┌─name─┬─type───┬─default_type─┬─default_expression─┬─comment─┬─codec_expression─┐
 │ url  │ String │              │                    │         │                  │
 │ from │ IPv6   │              │                    │         │                  │
@@ -19,19 +19,19 @@ DESCRIBE TABLE hits;
 
 OR you can use `IPv6` domain as a key:
 
-``` sql
+```sql
 CREATE TABLE hits (url String, from IPv6) ENGINE = MergeTree() ORDER BY from;
... MORE

@zhang2014
Copy link
Contributor Author

zhang2014 commented Oct 27, 2019

This script is based on the assumption that documents in other languages are fully synchronized with the en document at a commit.

@zhang2014 zhang2014 mentioned this pull request Oct 27, 2019
Merged
blinkov
blinkov suggested changes Oct 28, 2019
View reviewed changes
docs/tools/easy_diff.py Outdated
Copy link
Contributor

@blinkov blinkov Oct 28, 2019

Choose a reason for hiding this comment

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

Suggested change
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
Copy link
Contributor

@blinkov blinkov Oct 28, 2019

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

docs/tools/easy_diff.py Outdated
Copy link
Contributor

@blinkov blinkov Oct 28, 2019

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
Copy link
Contributor

@blinkov blinkov Oct 28, 2019

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.

@blinkov
Copy link
Contributor

blinkov commented Oct 28, 2019

This script is based on the assumption that documents in other languages are fully synchronized with the en document at a commit.

@zhang2014 it's better to add this comment to --help output or at least in code comments

@zhang2014 zhang2014 requested a review from blinkov October 29, 2019 06:21
@zhang2014 zhang2014 mentioned this pull request Nov 1, 2019
Closed
@alexey-milovidov alexey-milovidov merged commit b2f72c7 into ClickHouse:master Nov 1, 2019
@CurtizJ CurtizJ added the pr-documentation Documentation PRs for the specific code PR label Nov 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alexey-milovidov alexey-milovidov alexey-milovidov approved these changes

@blinkov blinkov Awaiting requested review from blinkov

Assignees

No one assigned

Labels

pr-documentation Documentation PRs for the specific code PR

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@zhang2014 @blinkov @alexey-milovidov @CurtizJ