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

feat: optionally exclude descriptions from tag search results #118

Merged
merged 2 commits into from
Feb 29, 2024
Merged

feat: optionally exclude descriptions from tag search results #118

merged 2 commits into from
Feb 29, 2024

Conversation

asmfstatoil
Copy link
Collaborator

@asmfstatoil asmfstatoil commented Jun 20, 2022

No description provided.

@einarsi
Copy link
Collaborator

einarsi commented Jun 21, 2022

Can you describe the added value in this feature? What is wrong with just doing as described here after getting the results if you are only interested in the tag?
#87

Also, this PR leaves the search method with two potental output formats: The original list[tuple[str, str]] and the new list[str]. I am not too keen on that since it can be confusing and can lead to issues. But I am open to being convinced otherwise.

@asmfstatoil
Copy link
Collaborator Author

I agree with the proposal in #87 , and actually never want the description when searching for tags. So for me the value is that I do not have to implement the proposed scrubbing in all other applications :)

For the second point, I thought that the need to specify an optional input value would alert the user that the output format is different.

@asmfstatoil
Copy link
Collaborator Author

ODBC is no longer relevant so I think this is good to go

@asmfstatoil
Copy link
Collaborator Author

@mortendaehli Can you review?

@mortendaehli mortendaehli merged commit 857a594 into equinor:main Feb 29, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants