-
Notifications
You must be signed in to change notification settings - Fork 22
Replace the "log" server module fn with a "log2" #522
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
Conversation
Please use emoji reactions ON THIS COMMENT to indicate your position on this proposal.
Here are the meanings for the emojis:
|
@neicker Does this look okay to you? |
Looks good to me. |
While looking at this I remembered that I had more issues with
Unclear to me if we want to handle this in this PR or if I should rather open a new issue? |
I'll try to address those points here - thx! |
@neicker I have updated the text to address your questions - please see if this is adequate or further clarification is required. |
@naughtont3 Conflicts resolved - should be good to go! |
Note from 25Q3 meeting, will allow more time for reviews/comments and bring for vote at next quarterly (25Q4). |
\pasteAttributeItem{PMIX_LOG_XML_OUTPUT} | ||
\pasteAttributeItem{PMIX_LOG_EMAIL} | ||
\pasteAttributeItem{PMIX_LOG_EMAIL_ADDR} | ||
\pasteAttributeItem{PMIX_LOG_EMAIL_SENDER_ADDR} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A number of these have been removed from this list but not reintroduced in another list, should they be added to the list of optional attributes above?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PMIX_LOG_EMAIL
is still in the list, just moved to the section immediately above this one. The other LOG_EMAIL_xxx
attributes are listed in the PMIX_LOG_EMAIL
description as they are to be included in the pmix_data_array_t
for that attribute. We can split them into a list within that description (as was done for the stats area) if that helps.
2025-Q4 Vote passed: 7 yes, 0 No, 0 Abstain |
@naughtont3 Can you please click off the review button? |
The current "log" server module function was defined with a void return. This contrasts with most of the other module functions which characteristically return a pmix_status_t, thereby supporting a return status that indicates if the function is not supported or has been atomically completed. Add a replacement "log2" function that is identical in signature except for returning a pmix_status_t instead of void. Deprecate the current "log" function. Signed-off-by: Ralph Castain <[email protected]> Provide better description for PMIx_Log arguments Expand on the use of the "data" vs "directives" attribute arrays, providing a couple of examples to help clarify their use. Signed-off-by: Ralph Castain <[email protected]>
Rebased without conflicts! |
The current "log" server module function was defined with a void return. This contrasts with most of the other module functions which characteristically return a pmix_status_t, thereby supporting a return status that indicates if the function is not supported or has been atomically completed.
Add a replacement "log2" function that is identical in signature except for returning a pmix_status_t instead of void. Deprecate the current "log" function.