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

Feature: Allow to set the verbosity of the debug #2523

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

JGodin-C2C
Copy link

Summary

Allow to set the verbosity of the debug

Copy link
Collaborator

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

I'm rather allergic to emojis in commit messages and would like to keep those out of the log. Can you drop it?

manifests/mod/security.pp Outdated Show resolved Hide resolved
@JGodin-C2C JGodin-C2C changed the title ✨ Allow to set the verbosity of the debug Feature: Allow to set the verbosity of the debug Feb 1, 2024
@JGodin-C2C JGodin-C2C requested a review from ekohl February 1, 2024 08:01
ekohl
ekohl previously approved these changes Feb 1, 2024
@ekohl ekohl added the feature label Feb 1, 2024
@JGodin-C2C
Copy link
Author

The tests errors does not seems to be related to my new code.

@JGodin-C2C
Copy link
Author

hello ! what is missing here to see this merged in the main branch ?

@smortex
Copy link
Collaborator

smortex commented Feb 27, 2024

The tests errors does not seems to be related to my new code.

Hey! This should have been fixed in the main branch.

Can you please rebase your changes on top of it?

From your working directory:

git fetch origin       # Download the latest code we have here
git rebase origin/main # Move your commits on top of the main branch
git push -f            # Send the changes (-f is required because we re-wrote history)

Copy link
Collaborator

@smortex smortex left a comment

Choose a reason for hiding this comment

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

One minor issue to check please.

manifests/mod/security.pp Outdated Show resolved Hide resolved
@bastelfreak
Copy link
Collaborator

@JGodin-C2C please take a look at your git history. you did a merge instead of a merge. Please do a proper rebase, as @smortex suggested, to remove the merge commit.

smortex
smortex previously approved these changes Feb 27, 2024
Copy link
Collaborator

@smortex smortex left a comment

Choose a reason for hiding this comment

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

LGTM, let's trigger another CI!

@JGodin-C2C
Copy link
Author

@JGodin-C2C please take a look at your git history. you did a merge instead of a merge. Please do a proper rebase, as @smortex suggested, to remove the merge commit.

Yep, sorry, juggling with different remote is sometime hazardous.

Rebase should be correct now.

smortex
smortex previously approved these changes Feb 27, 2024
Copy link
Collaborator

@smortex smortex left a comment

Choose a reason for hiding this comment

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

That feels good!

@JGodin-C2C
Copy link
Author

That feels good!

Thanks for taking time to review my Merge Request even tho i stumbled on the carpet on repeated time

@JGodin-C2C
Copy link
Author

Sorry :( i pushed to the main branch again. my bad.

@JGodin-C2C
Copy link
Author

Hey @smortex i hope it's okay to query you in that way.

Any way to see this approved and merged ?

@ekohl
Copy link
Collaborator

ekohl commented Apr 29, 2024

There's still a merge commit in there.

@JGodin-C2C
Copy link
Author

okay, so, i took the tests from this PR (kudos to @zzacharo ):
#2524
and rebased.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants