Improve clientmove function

[Sebbo94BY]

• Add support for moving multiple clients at once
• Add support for additional parameters

To keep the old functionality, I've implemented a logic to archive this, so that existing code of other people don't needs to be changed.

[Sebbo94BY]

Mhmm, I don't get this CodeQL error. Since it's a beta check and I haven't changed anything related to this mentioned error, I would say, my code should be fine. 😅

[Murgeye]

Thanks for contributing again!

The CodeQL error is not your fault, it's probably because the offending code is in the same file.

I will have a look at your changes tomorrow!

[Murgeye]

I think this is overall a good change, but I am unsure if renaming the client_id parameter does not break the API which would be a problem (in case someone used named parameters for the non-optional parameters), since this is one of the oldest implemented functions. On the other hand, keeping the name in singular would also be confusing. I am not sure how to apply this change yet.

[Sebbo94BY]

Yeah, difficult. 😅

I mean, when somebody updates this package in a project, the person should also take a look at the changes and latest then, this change should be visible.

Usually you also update things, test them and deploy them afterwards in production, so during testing they should get an error, that the parameter with the old name does not exist (anymore).

[Murgeye]

I like the addition of params (especially, since clientmove seems to have a channelpw parameter that I haven't implemented). However, I am unsure about this implementation:

1. I think the ability to additional parameters should be provided for all commands (in case the commands change over time)
2. I don't particularly like that it is just a string. I would rather work with **kwargs and build the string from that.

[Sebbo94BY]

Thanks for your feedback. I'll take a look at it asap.

[Sebbo94BY]

I had a look at your feedback.

I don't particularly like that it is just a string. I would rather work with **kwargs and build the string from that.

You are talking here about the client_ids, or? Because the params should be already in the respective format.

I'm not really sure, if it's really a good idea to change this here at the moment from my solution to something else. I decided myself for this solution to keep the backward compatibilty for other API users as it was. We could think about changing this overall in a new major version.

What do you think about this?

1. For now, only change this single function to support params.
2. Improve overall code with e.g. PyLint and Black (Formatter). I can provide Github workflows for this and adjust the code. This will just take some time as you have a lot of code here, which needs some small adjustments. 😅
3. Refactor / Improve existing code to support params for all functions (which support params). We could also add it to functions, which do not support params, but this doesn't make sense. Let's rather fail if too many params were given.
4. Release new major version.