quadlet: remove unused 'name' parameter from ConvertNetwork and ConvertVolume #27234
Conversation
openshift-ci
bot
added
release-note-none
do-not-merge/work-in-progress
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
pkg/systemd/quadlet/quadlet.go
Outdated
| // Also returns the canonical network name, either auto-generated or user-defined via the | ||
| // NetworkName key-value. | ||
| func ConvertNetwork(network *parser.UnitFile, name string, unitsInfoMap map[string]*UnitInfo, isUser bool) (*parser.UnitFile, error, error) { | ||
| func ConvertNetwork(network *parser.UnitFile, _ string, unitsInfoMap map[string]*UnitInfo, isUser bool) (*parser.UnitFile, error, error) { |
There was a problem hiding this comment.
If we don't need that arg let's just remove it and simplify the callers. We only need to use _ if we need the arg to satisfy an interface or some other function signature or need to keep API compatibility which should not be the case here.
pkg/systemd/quadlet/quadlet.go
Outdated
| // Also returns the canonical volume name, either auto-generated or user-defined via the VolumeName | ||
| // key-value. | ||
| func ConvertVolume(volume *parser.UnitFile, name string, unitsInfoMap map[string]*UnitInfo, isUser bool) (*parser.UnitFile, error, error) { | ||
| func ConvertVolume(volume *parser.UnitFile, _ string, unitsInfoMap map[string]*UnitInfo, isUser bool) (*parser.UnitFile, error, error) { |
…rtVolume The 'name' parameter was unused in both ConvertNetwork and ConvertVolume functions. Remove the parameter entirely and update all function calls accordingly. This fixes revive linter warnings: - pkg/systemd/quadlet/quadlet.go:961:47: unused-parameter: parameter 'name' seems to be unused - pkg/systemd/quadlet/quadlet.go:1050:45: unused-parameter: parameter 'name' seems to be unused Signed-off-by: Jan Rodák <[email protected]>
Honny1
changed the title
I removed the name parameter. It was simple. Let's wait for CI to finish to see if I broke something. |
openshift-ci
bot
added
the
approved
|
Thanks for the change. While you're at it, I can see that |
Considering this is blocking main and other PRs currently I don't think we should make @Honny1 do additional cleanup work beyond what the linter issue is about. This can always be fixed later if you want to change this. @containers/podman-maintainers PTAL this is needed to unblock CI |
|
@ygalblum I've added cleanup to my to-do list and will handle it in a separate PR. |
|
@Luap99 sure, didn't know it was blocking. But, how is |
|
ConvertPod has the arg defined as 090304a was using auto fixes and was quite large already so changing things manually to actually remove the args in cases where possible would have made the PR hard to review. |
|
@Luap99 Thanks. I can now see that I was looking at a branch that did not include that commit |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Honny1, Luap99, ygalblum The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/lgtm |
3 participants
Does this PR introduce a user-facing change?