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

Improve logger_test and remove outdated comments #601

Merged
merged 1 commit into from
Dec 23, 2023
Merged

Conversation

rdmark
Copy link
Member

@rdmark rdmark commented Dec 19, 2023

Improved logger_test:

  • log type identifiers should be lower case
  • Uncommended the syslog tests, and brought their logging API calls up to date
  • Added a switch to the default log scope when changing from syslog to file log, otherwise the file logging didn’t take effect.
  • Fleshed out and numbered the test case outputs

Additionally: removed inline documentation of the log API that was outdated. The code itself is readable enough to serve as documentation.

@rdmark rdmark requested a review from a user December 19, 2023 06:08
Copy link

sonarcloud bot commented Dec 19, 2023

@ghost
Copy link

ghost commented Dec 19, 2023

@rdmark, I'm probably missing something obvious here but when I compile and run logger_test on macOS it gives no visible output. Hmm...

@rdmark
Copy link
Member Author

rdmark commented Dec 19, 2023

@dgsga the first four tests should output to syslog, and the rest of them should output to a file called ./test.log

It’s probably a good idea to have the app output something to this effect to stdout.

Are you able to see logs in both places on macOS?

@ghost
Copy link

ghost commented Dec 20, 2023

@dgsga the first four tests should output to syslog, and the rest of them should output to a file called ./test.log

It’s probably a good idea to have the app output something to this effect to stdout.

Are you able to see logs in both places on macOS?

I get the test.log output but no changes in Console.app -> system.log
test.log

@rdmark
Copy link
Member Author

rdmark commented Dec 20, 2023

That’s curious. The syslogs show up on Debian.

I assume that syslogs generated by afpd can be seen on macOS? I don’t think I ever checked.

@ghost
Copy link

ghost commented Dec 22, 2023

That’s curious. The syslogs show up on Debian.

I assume that syslogs generated by afpd can be seen on macOS? I don’t think I ever checked.

No afpd generated syslog viewable on macOS. I have approved the PR so you can push if you're OK with this...

@rdmark
Copy link
Member Author

rdmark commented Dec 23, 2023

@dgsga I filed #615 for logging on macOS.

@rdmark rdmark merged commit 4756811 into main Dec 23, 2023
4 checks passed
@rdmark rdmark deleted the rdmark-issue-584 branch December 23, 2023 13:36
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.

1 participant