Skip to content
This repository was archived by the owner on May 17, 2024. It is now read-only.

Added colors to diff output #34

Merged
merged 2 commits into from
May 25, 2022
Merged

Added colors to diff output #34

merged 2 commits into from
May 25, 2022

Conversation

erezsh
Copy link
Contributor

@erezsh erezsh commented May 25, 2022

No description provided.

@erezsh erezsh requested a review from sirupsen May 25, 2022 08:37
@erezsh
Copy link
Contributor Author

erezsh commented May 25, 2022

Btw you can ignore whitespace in github too: https://github.com/datafold/data-diff/pull/34/files?diff=split&w=1

Copy link
Contributor

@sirupsen sirupsen left a comment

Choose a reason for hiding this comment

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

LGTM

@click.option("-v", "--verbose", is_flag=True, help="Print extra info")
@click.option("-i", "--interactive", is_flag=True, help="Confirm queries, implies --debug")
@click.option(
"-j", "--threads", default=None, help="Number of threads to use. 1 means no threading. Auto if not specified."
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't the default 1 thread? What does auto mean? I think it should be 1 so that the output is nice and expected in the default case. It's also far safer on a relational database where much more could really have an impact (especially with the current queries).

Sorry I know this is not strictly related here, but I must've missed this on the other PR. I thought I read for this when I reviewed the threading PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let's discuss it on slack.

@erezsh erezsh merged commit 5f9e3f6 into master May 25, 2022
nolar pushed a commit that referenced this pull request Apr 6, 2023
snowflake: add support for key password
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants