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

Show command output #115

Merged
merged 14 commits into from
Mar 22, 2022
Merged

Show command output #115

merged 14 commits into from
Mar 22, 2022

Conversation

jloleysens
Copy link
Contributor

Mainly a fix to always show relevant command output when appropriate. For example, when listing all credential definitions.

Also updated contributing documentation with reasoning for when to use what type of logging.

Copy link
Member

@berendsliedrecht berendsliedrecht left a comment

Choose a reason for hiding this comment

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

Nice additions. Did you test it all with ./tests/run.sh and some manual stuff?

@jloleysens
Copy link
Contributor Author

@blu3beri I did both :)

The last output of the tests/run.sh was error: {"connection_id": ["Not a valid UUID."]}

Is this expected?

@berendsliedrecht
Copy link
Member

@blu3beri I did both :)

The last output of the tests/run.sh was error: {"connection_id": ["Not a valid UUID."]}

Is this expected?

Mine does not do that. It means that the default output is not the connection id. U added a prefix to the print (connection id) in green. Couls you print that as an info?

@jloleysens
Copy link
Contributor Author

OK, i tested checking out the main branch (from where this branch started) and it looks like I still get that output.

@jloleysens
Copy link
Contributor Author

I'm going to go ahead and merge so long, happy to take on debugging tests in a follow up

@jloleysens jloleysens merged commit e074858 into main Mar 22, 2022
@jloleysens jloleysens deleted the fix/show-command-output branch March 22, 2022 11:49
@jloleysens jloleysens mentioned this pull request Mar 28, 2022
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