Move existing client script code to a better place

[okurz]

See discussion in #2933

[codecov]

Codecov Report

Merging #2985 into master will increase coverage by 0.01%.
The diff coverage is 70.87%.

@@            Coverage Diff             @@
##           master    #2985      +/-   ##
==========================================
+ Coverage   91.70%   91.72%   +0.01%     
==========================================
  Files         202      203       +1     
  Lines       12718    12722       +4     
==========================================
+ Hits        11663    11669       +6     
+ Misses       1055     1053       -2     

Impacted Files	Coverage Δ
lib/OpenQA/Client.pm	100.00% <ø> (+28.42%)	⬆️
lib/OpenQA/Script/CloneJob.pm	85.24% <ø> (ø)
lib/OpenQA/WebAPI/Plugin/YAML.pm	100.00% <ø> (ø)
script/openqa-clone-job	54.62% <25.00%> (ø)
lib/OpenQA/Script/Client.pm	71.57% <71.57%> (ø)
lib/OpenQA/Setup.pm	96.15% <100.00%> (ø)
script/client	84.21% <100.00%> (+0.87%)	⬆️
lib/OpenQA/Worker/CommandHandler.pm	93.75% <0.00%> (+1.56%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 75468e7...ae5f739. Read the comment docs.

[Martchus]

I'd still prefer this under a new module to prevent the "script" module from growing too big. And I generally don't think it is bad to add relevant version components to a (library) name. It would just be nice not to mix messy code with other potentially messy code or simply code which previously had not business with that client code.

[kalikiana]

I'd still prefer this under a new module to prevent the "script" module from growing too big. And I generally don't think it is bad to add relevant version components to a (library) name. It would just be nice not to mix messy code with other potentially messy code or simply code which previously had not business with that client code.

Especially if the PR title is insinuating that the Script module is a code graveyard ;-)

I'm still unclear what the long-term plan is with this code. What is the would be new module going to be about? Is this a "Legacy" module? Or a "ClientV1"? What else will live in it?

[okurz]

I have the same questions as you do :)

Script was a suggestion from @kraih . If he prefers Script.pm I would use that. Another name would also be ok. As I stated about "old" already in before, the same applies to "legacy", it becomes ambiguous soon. One can say every code becomes "legacy" as soon as it reaches master ;)

[Martchus]

The name isn't that important. It would be useful to have an explaining comment like # routines used by the openqa-client script anyways. So maybe OpenQA::Script::Client? The new "client" is actually called "cli" (script name and namespace-wise) so it wouldn't be too confusing. (And if we'd ever wanted to do the same for the clone script specific functions could go into OpenQA::Script::CloneJob. That's better than having all extra functions for all scripts in one big module. So let's don't do the first step in that direction.)

[kraih]

Agree about OpenQA::Script::CloneJob, that's better than a generic OpenQA::Script with random functions used by various scripts. Regarding OpenQA::Client, that module was really clean before #2933, and had a clearly defined scope (might have even been worth merging with OpenQA::UserAgent in an authentication code cleanup). There was simply no upside at all to moving script code into that namespace.

[okurz]

Ok, so my proposal is based on current master:

• Move current functions in OpenQA::Scripts in OpenQA::Scripts::CloneJob, leave OpenQA::Scripts empty
• Move functions used by script/client from OpenQA::Client to OpenQA::Scripts::Client

ok?

[Martchus]

Yes, that seems to be the best solution I can currently think of.

[kraih]

I would have preferred if the code had not been moved in the first place.

[okurz]

We know