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

Propose marking members as non-public and moving public AuthenticationError #162

Closed
elliot-100 opened this issue Oct 22, 2024 · 1 comment · Fixed by #163
Closed

Propose marking members as non-public and moving public AuthenticationError #162

elliot-100 opened this issue Oct 22, 2024 · 1 comment · Fixed by #163
Assignees
Labels
enhancement New feature or request

Comments

@elliot-100
Copy link
Collaborator

elliot-100 commented Oct 22, 2024

I think these members shouldn't be used by end-users and therefore should be prefixed with _:

spond.spond.Spond.chat_url
spond.spond.Spond.auth
spond.spond.Spond.login_chat()

I think this one should be moved:

spond.base.AuthenticationError -> spond.AuthenticationError

as spond.base should only contain the internal _SpondBaseclass.

I am not sure if this should be considered a breaking change.

But once we have done this, I think we probably have the correct public API, and so will make it clearer in future:

  • what should be properly documented
  • what is a breaking change/functional addition/removal (as it affects part of the public API)
  • what can be freely refactored/removed (as it only affects internals).
@elliot-100 elliot-100 added the enhancement New feature or request label Oct 22, 2024
@elliot-100 elliot-100 self-assigned this Oct 22, 2024
@elliot-100 elliot-100 changed the title Propose marking members as non-public Propose marking members as non-public and moving public AuthenticationError Oct 22, 2024
@Olen
Copy link
Owner

Olen commented Oct 22, 2024

I think it is pretty obvious that those functions are not meant to be public, so if anyone have used them in 3. party code, they should not be surprised if they change.
So to me it sounds reasonable to prefix them with _ and don't worry about breaking anything.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants