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 base logger #73

Merged
merged 10 commits into from
Mar 3, 2023
Merged

feat: add base logger #73

merged 10 commits into from
Mar 3, 2023

Conversation

tony-go
Copy link
Member

@tony-go tony-go commented Feb 16, 2023

Fixes: #71

Copy link
Contributor

@RaisinTen RaisinTen left a comment

Choose a reason for hiding this comment

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

Are these log formats consistent with the format LIEF uses in the LIEF_ERR, LIEF_WARN, etc macros?

src/cli.js Show resolved Hide resolved
src/cli.js Show resolved Hide resolved
vendor/lief/src/logging.hpp Outdated Show resolved Hide resolved
Copy link
Contributor

@RaisinTen RaisinTen left a comment

Choose a reason for hiding this comment

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

The next step would be to add a patch file to patches/lief for the changes that have been made to vendor/lief. You can run git format-patch -1 --zero-commit --relative in the vendor/lief directory to generate the patch file and move it to the location and commit it.

vendor/lief/src/logging.hpp Outdated Show resolved Hide resolved
Copy link
Contributor

@RaisinTen RaisinTen 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 this locally on macOS and Linux and it LGTM. Didn't get a chance to test on Windows, although I believe @fraxken has already done that (thanks once again!).

@RaisinTen RaisinTen merged commit 09fcff0 into main Mar 3, 2023
@RaisinTen RaisinTen deleted the add-logger branch March 3, 2023 06:14
RaisinTen added a commit that referenced this pull request Mar 3, 2023
Originated in #73.

Signed-off-by: Darshan Sen <raisinten@gmail.com>
RaisinTen added a commit that referenced this pull request Mar 3, 2023
Originated in #73.

Signed-off-by: Darshan Sen <raisinten@gmail.com>
RaisinTen added a commit that referenced this pull request Mar 3, 2023
#73 uncommented an assertion
which is known to fail on linux, so comment it once again.

Signed-off-by: Darshan Sen <raisinten@gmail.com>
@RaisinTen RaisinTen mentioned this pull request Mar 3, 2023
RaisinTen added a commit that referenced this pull request Mar 3, 2023
#73 uncommented an assertion
which is known to fail on linux, so comment it once again.

Signed-off-by: Darshan Sen <raisinten@gmail.com>
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.

Print a message when the build is successful
4 participants