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

Feature: Make defaultTimeout property a variable instead of a constant. #1356

Closed

Conversation

zachsimone
Copy link
Contributor

Description & motivation

In a project I'm currently working on, we've found it handy to be able to override the defaultTimeout property which is currently defined as a constant.

Currently we are forking the library and have made this modification there, however it's the last remaining change that we need the fork for. So, I am proposing to update the main library and we can do away with maintaining a custom fork.

I'm not suggesting it's advisable to change the value of this property necessarily, and I respect that if you'd rather leave it as a constant, but it works for a specific need we have and I thought I'd propose making the change in the main library as well since it's purely an additive change that allows users of the library additional flexibility without breaking any existing code.

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality [to the library])

@shogo4405
Copy link
Owner

I have defined a public variable called timeout and provided a means to modify it. Please consider the initial value of this variable as defaultTimeout. HaishinKit is consistently designed with the concept of default variables as described above.

The design is for setting RTMPConnection.timeout, so this pull request cannot be accepted.

@shogo4405 shogo4405 closed this Dec 15, 2023
@zachsimone
Copy link
Contributor Author

The design is for setting RTMPConnection.timeout

Very helpful, thank you for the reply. I'd missed this, but I'll adjust to setting this property instead. Thanks again!

@zachsimone zachsimone deleted the feature/settable-property branch December 16, 2023 02:48
@shogo4405
Copy link
Owner

After reviewing the code, I found that it crashes until RTMPConnection.connect is called. I have fixed it on my end. #1360

@zachsimone
Copy link
Contributor Author

@shogo4405 Thank you!

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