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.

Sign up for GitHub

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

Jump to bottom

feat: adding scaffold for concatenate as a cli operation #566

Merged
merged 14 commits into from
Oct 6, 2022
Merged

feat: adding scaffold for concatenate as a cli operation #566

merged 14 commits into from
Oct 6, 2022

Conversation

renzokuken
Copy link
Collaborator

No description provided.

@renzokuken renzokuken changed the title adding scaffold for cocnatenate as a cli operation feat: adding scaffold for cocnatenate as a cli operation Aug 23, 2022
@conventional-commit-lint-gcf
Copy link

🤖 I detect that the PR title and the commit message differ and there's only one commit. To use the PR title for the commit history, you can use Github's automerge feature with squashing, or use automerge label. Good luck human!

-- conventional-commit-lint bot
https://conventionalcommits.org/

@renzokuken
Copy link
Collaborator Author

/gcbrun

@renzokuken renzokuken changed the title feat: adding scaffold for cocnatenate as a cli operation feat: adding scaffold for concatenate as a cli operation Sep 9, 2022
@renzokuken
Copy link
Collaborator Author

/gcbrun

@renzokuken
Copy link
Collaborator Author

/gcbrun

nehanene15
nehanene15 requested changes Sep 15, 2022
View reviewed changes
data_validation/config_manager.py Show resolved Hide resolved
data_validation/cli_tools.py Show resolved Hide resolved
data_validation/__main__.py Outdated Show resolved Hide resolved
data_validation/__main__.py Outdated Show resolved Hide resolved
@renzokuken
Copy link
Collaborator Author

/gcbrun

nehanene15
nehanene15 reviewed Sep 16, 2022
View reviewed changes
README.md Outdated Show resolved Hide resolved
data_validation/__main__.py Show resolved Hide resolved
data_validation/__main__.py
@@ -131,11 +131,23 @@ def get_calculated_config(args, config_manager):
col_list = None if args.hash == "*" else cli_tools.get_arg_list(args.hash)
fields = config_manager._build_dependent_aliases("hash", col_list)
aliases = [field["name"] for field in fields]

# config_manager.append_dependent_aliases(aliases)
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: remove comment

data_validation/__main__.py Show resolved Hide resolved
renzokuken and others added 4 commits September 19, 2022 14:43
@renzokuken @nehanene15
Update README.md
54d9e1b 
Co-authored-by: Neha Nene <[email protected]>
@renzokuken
updating check for args
65d0182
@renzokuken
linting
93ac03a
@renzokuken
Merge branch 'concat_support_calc_fields' of github.com:GoogleCloudPl…
4166e5a 
…atform/professional-services-data-validator into concat_support_calc_fields
@renzokuken
Copy link
Collaborator Author

/gcbrun

nehanene15
nehanene15 approved these changes Oct 6, 2022
View reviewed changes
Copy link
Collaborator

@nehanene15 nehanene15 left a comment

Choose a reason for hiding this comment

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

LGTM! Only thing that may be good to add is some documentation on the difference between concat and hash (and how only one of the flags should be provided). I'm worried users may try to use both flags at once unaware that the --hash includes a concat.

@renzokuken
Copy link
Collaborator Author

/gcbrun

@renzokuken renzokuken merged commit ec4ef33 into develop Oct 6, 2022
@renzokuken renzokuken deleted the concat_support_calc_fields branch October 6, 2022 22:23
@release-please release-please bot mentioned this pull request Oct 6, 2022
Merged
@nehanene15 nehanene15 mentioned this pull request Oct 10, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nehanene15 nehanene15 nehanene15 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@renzokuken @nehanene15