fix: replace ptr.To("") by new(string)

[rxinui]

Changes

I tried to have a conversation with @HeavyWombat regarding possible changes or implementations. Here's a proactive PR.

I'm not certain if this solves the issue reported, let's discuss about it.

Fixes #310

Submitter Checklist

• Includes tests if functionality changed/was added
• Includes docs if changes are user-facing
• Set a kind label on this PR
• Release notes block has been filled in, or marked NONE

See the contributor guide
for details on coding conventions, github and prow interactions, and the code review process.

Release Notes

NONE

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign heavywombat for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[HeavyWombat]

I am not sure why there are two E2E tests in pending still.

[HeavyWombat]

/lgtm

[rxinui]

@HeavyWombat Hi

There was a discussion weeks ago about this PR. I've explained that changing ptr.To("") to new() does not provide any values as under the hood, the result is the same.

Instead, we were thinking of finding a way to replace the "Sanitize" process and avoid pointer of string. See https://docs.google.com/document/d/10tDPl_t2-7NcxmI1Iy50dIGPIRIdIUh8v6qp1iH3wLc/edit?tab=t.0#heading=h.cxmoz937fi39

[Nui] - #313
Discussion about the general two options we have:
Like now: we have flag values directly going into the object which means we afterwards have to cleanup optional fields in the Sanitize function
Possible change: flags store values in local variables and then we have a longer block of ifs that build the object based on which flags have been set or not
Nui plans to spend some time to see how option (2) looks like

[HeavyWombat]

@HeavyWombat Hi

There was a discussion weeks ago about this PR. I've explained that changing ptr.To("") to new() does not provide any values as under the hood, the result is the same.

Instead, we were thinking of finding a way to replace the "Sanitize" process and avoid pointer of string. See https://docs.google.com/document/d/10tDPl_t2-7NcxmI1Iy50dIGPIRIdIUh8v6qp1iH3wLc/edit?tab=t.0#heading=h.cxmoz937fi39

[Nui] - #313
Discussion about the general two options we have:
Like now: we have flag values directly going into the object which means we afterwards have to cleanup optional fields in the Sanitize function
Possible change: flags store values in local variables and then we have a longer block of ifs that build the object based on which flags have been set or not
Nui plans to spend some time to see how option (2) looks like

Thanks for hint. I wasn't aware. Should we close this PR then for the time being?

[rxinui]

I haven't find time to do it. Will spend some time next week. :)

[adambkaplan]

/hold

I don't think this solves the root issue at hand (why do we need pointers in the first place?). Instead I fear this change obfuscates the root issue when reading the code. A new golang developer may not be aware that new(string) produces a pointer, and not a string value. ptr.To("") makes this relationship explicit.