8000 cli: add verbose logging with `--debug` flag by osintalex · Pull Request #809 · edgelesssys/constellation · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

cli: add verbose logging with --debug flag #809

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
Merged
merged 11 commits into from
Jan 4, 2023
Merged

cli: add verbose logging with --debug flag #809

merged 11 commits into from
Jan 4, 2023

Conversation

osintalex
Copy link
Contributor
@osintalex osintalex commented Dec 16, 2022 •
edited by katexochen
Loading

Proposed change(s)

  • Introduce a debug flag that allows a user to turn on verbose debug logging

Related issue

Additional info

  • I've struggling to fully test this locally while working on mac since most of the documentation seems to be for linux. However I have run the test suite and made sure that passes.

Checklist

  • Update docs I don't think they need updating since the debug flag is already there? But happy to add stuff if needed!
  • Add labels (e.g., for changelog category) - don't think I have permissions for this
  • Link to Milestone - don't think I have permissions for this

@netlify
Copy link
netlify bot commented Dec 16, 2022 •
edited 8000
Loading

✅ Deploy Preview for constellation-docs canceled.

Name Link
🔨 Latest commit d9b3a10
🔍 Latest deploy log https://app.netlify.com/sites/constellation-docs/deploys/63b4efdad5aa2e000933050b

@daniel-weisse daniel-weisse added this to the v2.4.0 milestone Dec 19, 2022
@daniel-weisse daniel-weisse added the feature This introduces new functionality label Dec 19, 2022
@daniel-weisse daniel-weisse requested review from katexochen and daniel-weisse and removed request for katexochen December 19, 2022 07:58
daniel-weisse
daniel-weisse reviewed Dec 19, 2022
View reviewed changes
Copy link
Member
@daniel-weisse daniel-weisse 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 from my side.
Just some minor things:

@daniel-weisse
Copy link
Member

Rebasing should fix the unit tests not running correctly 👍

katexochen
katexochen reviewed Dec 23, 2022
View reviewed changes
Copy link
Member
@katexochen katexochen left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution, and sorry again for our CI, we're working on improving this for external contributors. 😉 Most tests, passed, and two of the failed ones failed due to token issues on our side. I've checked your PR manually, and there are only two little things need to be changed regarding passing the CI.

@osintalex
Copy link
Contributor Author

No worries and my bad - I forgot to run go fmt on those files I think.

I also meant to ask, would you like me to rebase to remove the merge commits from main?

I haven't actually rebased this branch yet, just merged in the latest commits from main.

katexochen
katexochen requested changes Dec 28, 2022
View reviewed changes
Copy link
Member
@katexochen katexochen left a comment

Choose a reason for hiding this comment

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

I've tested the current state, works great! :)

Found some things that are sensitive and shouldn't be logged, see review comments.

Guess we can merge after these are fixed.

@katexochen
Copy link
Member

would you like me to rebase to remove the merge commits from main?

No need to rebease, we just wanted to get the CI running. The commits of your PR will be squash-merged, so no need to tidy up the history.

@katexochen katexochen self-assigned this Dec 28, 2022
@katexochen katexochen changed the title Introduce debug flag v2 cli: add verbose debug logging Jan 4, 2023
katexochen
katexochen approved these changes Jan 4, 2023
View reviewed changes
Copy link
Member
@katexochen katexochen left a comment

Choose a reason for hiding this comment

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

lgtm

@katexochen katexochen merged commit 97c72f5 into edgelesssys:main Jan 4, 2023
@katexochen katexochen mentioned this pull req 82BB uest Jan 4, 2023
Closed
@thomasten thomasten changed the title cli: add verbose debug logging cli: add verbose logging with --debug flag Jan 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@daniel-weisse daniel-weisse daniel-weisse left review comments

@katexochen katexochen katexochen approved these changes

Assignees

@katexochen katexochen

Labels
feature This introduces new functionality
Projects
None yet
Milestone
v2.4.0
Development

Successfully merging this pull request may close these issues.
3 participants
@osintalex @daniel-weisse @katexochen
0