added prival function #323
Conversation
There was a problem hiding this comment.
It looks like you're making good progress! I've left some specific comments for you in the review. In addition to those, here are some more general points:
- make sure that you add your new
.cfile to the CMakeLists.txt file (in theSTUMPLESS_SOURCESlist) so that it is included during compilation. - You'll need to add an entry into
src/windows/stumpless.defwith your new function so that it is usable from Windows systems. - You'll need to add your new function name to
tools/check_headers/stumpless.ymlas well so that it is used during header checks.
|
Thank for review. I little edited function |
|
It looks like there are some compilation issues, as well as unused includes in the new |
CMakeLists.txt
Outdated
| @@ -1218,6 +1220,10 @@ add_function_test(perror | |||
| SOURCES test/function/startup/perror.cpp | |||
| ) | |||
|
|
|||
| add_function_test(facility | |||
There was a problem hiding this comment.
you'll need to change the name of this test from facility to something unique (probably prival is the best choice) in order for CMake to work properly.
include/stumpless/prival.h
Outdated
| # define __STUMPLESS_PRIVAL_H | ||
|
|
||
| # include <stumpless/config.h> | ||
| # include <stumpless/generator.h> |
There was a problem hiding this comment.
use the header check tool (or the results of the Static Analysis CI check) to identify which include statements aren't needed - this one is unneeded at a minimum.
test/function/prival.cpp
Outdated
| TEST( GetPrivalString, ValidPrival ) { | ||
| const char *result; | ||
|
|
||
| #define CHECK_PRIVAL_TRUE( STRING, INT ) \ |
There was a problem hiding this comment.
this is just a #define, and doesn't appear to actually run any tests?
|
I made the changes you described, please check. |
|
You are on a good path, but there are still some compilation issues that you will need to work through locally. Make sure that you can build the code and run the tests on your own machine before pushing to this PR. Also, be careful with your pointers: the code as is will cause a segfault, as it writes into a pointer that has not been set to a valid address. You'll need to allocate memory via |
|
I fixed memory and other things, now should build run well. |
|
Can you please launch test again? I pushed two commits but and it tested not the last one, but the one, whitch I was pushed just moment before it. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
I fix test, which causes an error. Can you please run tests again? |
|
I added heading file, which was missing in test, can you try launch test now? Thanks. |
|
I've gotten the ANSI-only build support to a state where the library and tests compile, though the network tests and some static analysis still fails at the moment. Please merge in the |
|
Checking in to see if you intend to continue work on this, per the contribution guidelines. |
|
Closed as stale. |
added function
stumpless_get_prival_string