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

server can override and add custom command mappings #177

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rupurt
Copy link

@rupurt rupurt commented May 14, 2024

What do these changes do?

The Server class can override and add custom command mappings. I'm submitting this as a draft PR without unit tests to open the conversation around if this is a desirable change that will be accepted.

Are there changes in behavior for the user?

No

Related issue number

None

Checklist

  • I think the code is well written
  • Unit tests for the changes exist
  • Documentation reflects the changes

@rupurt rupurt force-pushed the server-can-add-custom-commands-mapping branch from 361fa63 to 4d8398a Compare December 18, 2024 03:48
@rupurt
Copy link
Author

rupurt commented Dec 23, 2024

@pohmelie do you have any suggestions to get this merged?

@pohmelie
Copy link
Collaborator

@pohmelie do you have any suggestions to get this merged?

You need to move this from draft. "Draft" mode implies you are not ready for code review.

image

@rupurt rupurt marked this pull request as ready for review December 23, 2024 20:59
@rupurt
Copy link
Author

rupurt commented Dec 23, 2024

Done! 🍻

setup.py Outdated
from setuptools import setup

if __name__ == "__main__":
setup()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually we do not need setupy.py, because of pyproject.toml file

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I originally thought I needed it for Nix but I found an override. I'll remove the commit

@@ -660,6 +660,7 @@ def __init__(
data_ports=None,
encoding="utf-8",
ssl=None,
extra_commands_mapping={},
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This mapping should be added to documentation

@rupurt rupurt force-pushed the server-can-add-custom-commands-mapping branch from 4d8398a to 1308c03 Compare December 23, 2024 23:07
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.

2 participants