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.

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 table name option to | teragrep exec bloom #87

Merged
merged 2 commits into from
Nov 14, 2024

Conversation

51-code
Copy link
Contributor

@51-code 51-code commented Nov 13, 2024

Related to PTH-10 issue 410.

Adds an optional parameter for the table name. PTH-10 is responsible for parsing and will throw an exception with a descriptive message if the table name is not given.

@51-code 51-code added the enhancement New feature or request label Nov 13, 2024
@51-code 51-code requested a review from eemhu November 13, 2024 08:01
Fix typo in bloomOptionParameter

Add table name option to | teragrep exec bloom
@51-code 51-code marked this pull request as draft November 13, 2024 08:47
@51-code
Copy link
Contributor Author

51-code commented Nov 13, 2024

Will be adding regex to the command as well, converted to a draft PR.

@51-code 51-code removed the request for review from eemhu November 13, 2024 08:48
@51-code 51-code marked this pull request as ready for review November 14, 2024 07:20
@51-code 51-code requested a review from eemhu November 14, 2024 07:20
Copy link
Contributor

@eemhu eemhu left a comment

Choose a reason for hiding this comment

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

Looks good to me, however perhaps a new issue could be made of unifying the parameters to include = signs like in other commands. Also, the order of parameters must match the grammar and can't be given in any other order.

@51-code 51-code requested a review from kortemik November 14, 2024 07:46
@51-code 51-code added the review label Nov 14, 2024
@elliVM
Copy link

elliVM commented Nov 14, 2024

#88 Issue about parameter = signs

@kortemik kortemik merged commit 7ea31c1 into teragrep:main Nov 14, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants