goatshriek / stumpless

a C logging library built for high performance and a rich feature set
https://goatshriek.github.io/stumpless
Apache License 2.0
440 stars 319 forks source link

factor config_open_default_target out of wrapper.h #293

Closed goatshriek closed 11 months ago

goatshriek commented 1 year ago

Stumpless uses a scheme involving #definenames and macro functions to separate its portability measures from the application logic itself. This was originally done via a single header, include/private/config/wrapper.h, but as the library grew this single header became bloated. Now features are instead contained in their own dedicated wrapper headers. Older features and functions need to be moved to their own as well to address the original problem.

Your task here is to factor the function used to open the default target into it's own header. This is a good task for those with some C knowledge looking to learn about how to implement portability in a C application. As this is a refactoring task you can rely on the existing unit and integration tests as a safety net as you make changes and explore.

Check out the Contributing Guidelines and the development guide for the basics on working with stumpless and submitting changes.

General Approach

There are a few details left out of the following approach, for you to fill in as you encounter them. If you find you need help, please ask here or on the project gitter and someone can help you get past the stumbling block.

97698be5b62c3fd1dddfd953db15a5c7265b896c is an implementation of a very similar task: you can use this commit as a template for this task.

First, read through the portability documentation to understand how portability is approached in the library.

Next, have a look at include/private/config/wrapper/wstring.h. This header does the same wrapping for wide string support functions in the library. This is a great file to copy as a starting point and modify to suit your needs. The new header should be include/private/config/wrapper/open_default_target.h.

Finally, have a look at tools/check_headers/stumpless_private.yml. This file is used by a custom tool for header inclusion checks across the library's source code. You'll need to add an entry for config_open_default_target to this file, pointing to your newly created header.

Once you have moved things to your new header and updated the header checking tool manifests, you can use the header check tool to tell you what files need to have their #include statements updated. You can find examples of how to run the tool in the testing workflow that does it, .github/workflows/analysis.yml, or you can use the one-liner included below. You will need Ruby to run the tool.

tools/check_headers/check_headers.rb "include/**/*.h*" "include/**/**.h*" "src/**/*.c" "test/**/*.cpp" "docs/examples/**/*.c"

Good luck!

sivasuriyankumarasamy commented 1 year ago

Hi @goatshriek,

can you assign this to me.

sivasuriyankumarasamy commented 1 year ago

Hi @goatshriek Raised pr for this. please review it. Thanks

goatshriek commented 1 year ago

This issue has been re-opened for others to work due to inactiity.

rmknan commented 11 months ago

Hello, I would like to work on this. Please assign it to me.

goatshriek commented 11 months ago

The header check utility is going through the project headers and has found two with supposed violations: the open_default_target.h and private/config/wrapper.h headers.

Based on the message about open_default_target.h, it looks like the entry in stumpless.yml doesn't quite match the actual include name: note the extra config_ at the beginning of the filename. Updating that entry should remove those errors.

You can delete private/config/wrapper.h completely, as this refactoring was the final step in removing it. 🥳

Deprecation warnings can be ignored in this case.

rmknan commented 11 months ago

The open_default_target.h problem is solved but even if I remove all private/config/wrapper.h, it still says unused include private/config/wrapper.h. Is there any other file from which it has to be removed?

goatshriek commented 11 months ago

Does the error not specify the file it was found in?