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: add support for otp fields #61

Closed
wants to merge 1 commit into from

Conversation

fzipi360
Copy link
Contributor

what

  • add support for otp fields

why

  • there is no support for otp as it requires a specific flag to be passed to 1pass cli

Signed-off-by: Felipe Zipitria <fzipitria@life360.com>
Copy link
Collaborator

@dtpryce dtpryce left a comment

Choose a reason for hiding this comment

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

I am guessing this is always just a single line string so should be more than fine, but can you add a test for this new method? Just a very basic on mocking the other get methods please!

@fzipi360
Copy link
Contributor Author

🤔 from what I see here, there is not much room for mocking there... unless creating everything from scratch.

@dtpryce
Copy link
Collaborator

dtpryce commented May 28, 2024

🤔 from what I see here, there is not much room for mocking there... unless creating everything from scratch.

yeah it's nothing special since we are just wrapping the cli happy for it be:

def test_get_item_otp(self):
        """
        Tested in read_bash_return.
        """
        pass

Just for completeness

@dtpryce
Copy link
Collaborator

dtpryce commented May 28, 2024

🤔 from what I see here, there is not much room for mocking there... unless creating everything from scratch.

yeah it's nothing special since we are just wrapping the cli happy for it be:

def test_get_item_otp(self):
        """
        Tested in read_bash_return.
        """
        pass

Just for completeness

ok I will test that this works but no reason why it wouldn't and add the test for you so you can focus on the comma within fields PR.

@dtpryce
Copy link
Collaborator

dtpryce commented May 28, 2024

Merged in #62 ... will release soon

@dtpryce dtpryce closed this May 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants