-
Notifications
You must be signed in to change notification settings - Fork 126
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
Add support for kickstart
to mrack
plugin
#3064
Conversation
fc1f4a7
to
59d8e86
Compare
59d8e86
to
569a5b1
Compare
569a5b1
to
9e9b599
Compare
I would suggest finding a different name and adding a docstring
Thanks for your mentor,How about"normalize_dict"? :)
…On Fri, Aug 30, 2024 at 3:52 PM Miloš Prchlík ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In tmt/utils/__init__.py
<#3064 (comment)>:
> @@ -78,6 +78,34 @@
from tmt._compat.typing import Self, TypeAlias
+def _normalize_user_data(
By moving into tmt.utils:
- the function is no longer "private", used by artemis plugin and no
one else, therefore it should no longer start with an underscore,
- "user data" is a concept understood by Artemis and therefore by
artemis, but it does not mean much to the rest of the code base. I
would suggest finding a different name and adding a docstring. It was clear
what was happening when it was in tmt.steps.provision.artemis, but
what is tmt.utils._normalize_user_data doing? tmt does not accept any
"user data"...
—
Reply to this email directly, view it on GitHub
<#3064 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AKFR23HWCB3SWRYZRIOD4E3ZUAQENAVCNFSM6AAAAABKFAS64WVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDENZRGQ4DINZWGQ>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Still way too generic, I'm afraid, it can mean anything, any input, any type of keys and values, who knows, it's very open. We need a name that reflects it is normalizing |
9e9b599
to
ce85ac2
Compare
Updated^^ |
ce85ac2
to
6d49a21
Compare
@skycastlelily I tried to polish it a bit, I was not very happy about I also change the name of that normalization callback, to |
WDYT?
I really think it's brilliant, thanks do me the favor to co-author this
merge request and make the code look pretty while I'm on vacation^^
…On Mon, Sep 30, 2024 at 5:26 PM Miloš Prchlík ***@***.***> wrote:
@skycastlelily <https://github.com/skycastlelily> I tried to polish it a
bit, I was not very happy about beaker field, that leaks Mrack's weird
API into our code, so to_mrack() now exists. Check 466b568
<466b568>,
WDYT?
I also change the name of that normalization callback, to
normalize_string_dict, and I'm pretty happy about it, because it's what
it does, normalizes two different forms of key/value inputs into a dict[str,
str] :)
—
Reply to this email directly, view it on GitHub
<#3064 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AKFR23GELSXDBX2P7GDHMXDZZEKLJAVCNFSM6AAAAABKFAS64WVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGOBSGU4TQOJWHE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, thanks for implementing this!
* better formatting when logging kickstart * `normalize_string_dict` - no "user data" relicts, generalized docstring * use `kickstart` field in our code, translate for mrack when passing the barrier
Updated spec and added a release note in 5220d37. |
Thanks:)
…On Wed, Oct 2, 2024 at 4:10 AM Petr Šplíchal ***@***.***> wrote:
Merged #3064 <#3064> into main.
—
Reply to this email directly, view it on GitHub
<#3064 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AKFR23F32IHZSXITGDCTRH3ZZL6S5AVCNFSM6AAAAABKFAS64WVHI2DSMVQWIX3LMV45UABCJFZXG5LFIV3GK3TUJZXXI2LGNFRWC5DJN5XDWMJUGQ3TQOJTGA2DMMA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Co-authored-by: Miloš Prchlík <mprchlik@redhat.com> Co-authored-by: Petr Šplíchal <psplicha@redhat.com>
Pull Request Checklist