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

rfc42: rename flags to forwarding_flags #428

Closed
wants to merge 1 commit into from

Conversation

chu11
Copy link
Member

@chu11 chu11 commented Oct 10, 2024

Problem: The exec request uses a field named "flags" to indicate what should be forwarded to the client. The use of the generic term "flags" can be ambiguous, as it could be easily confused for other "flags" used within subprocess code.

Rename the field to "forwarding_flags".


side note, this is mostly b/c in the future we would like to pass the actual "subprocess" flags to the remote server. So this is just mini-prep for that.

Obviously can debate the name. I considered "callback_flags" as well.

Problem: The exec request uses a field named "flags" to indicate
what should be forwarded to the client.  The use of the generic
term "flags" can be ambiguous, as it could be easily confused
for other "flags" used within subprocess code.

Rename the field to "forwarding_flags".
@grondo
Copy link
Contributor

grondo commented Oct 10, 2024

Sorry if this was discussed elsewhere, but should we take a step back before breaking the protocol? Would it make sense to keep the current rexec flags and add a new rexec_flags member or mask the subprocess flags into the current flags member, rather than renaming?

Maybe my point is moot because we're breaking the protocol anyway?

@chu11
Copy link
Member Author

chu11 commented Oct 10, 2024

Sorry if this was discussed elsewhere, but should we take a step back before breaking the protocol? Would it make sense to keep the current rexec flags and add a new rexec_flags member or mask the subprocess flags into the current flags member, rather than renaming?

Yeah, I guess I should have said that this is 100% optional. Nothing stops us from leaving it as is and just adding a new field for the subprocess flags.

We can leave things as is for now. If we are going to break protocol, then we can reconsider this.

@garlick
Copy link
Member

garlick commented Oct 10, 2024

I'd vote for adding something like xflags (for "extended" flags) and leaving the existing flags as is.

@chu11
Copy link
Member Author

chu11 commented Oct 15, 2024

per discussion, closing

@chu11 chu11 closed this Oct 15, 2024
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.

3 participants