Fix import from old org #106

hoshsadiq · 2025-04-11T11:27:15Z

Description

Since the tool was moved from the old wayfair-incubator org to commercetools, using go install fails. This will fix the import to use the new org. Fixes #105

Type of Change

Checklist

I have read the contributing guidelines
Existing issues have been referenced (where applicable)
I have verified this change is not present in other open pull requests
Functionality is documented
All code style checks pass
New code contribution is covered by automated tests
All new and existing tests pass

Oded-B · 2025-04-11T14:14:30Z

The ERROR: invalid tag "/:pr-106": invalid reference format failure in Docker / build (pull_request) is unrelated this change - there's some bug in CI that fails PR from forks, I'll take a look

Oded-B · 2025-04-11T15:12:42Z

I'm 74% sure #108 will fix the Docker / build (pull_request failure

Oded-B · 2025-04-14T15:13:31Z

Alright #108 was merged, try to merge from main to see if this fixes the Docker / build (pull_request) issue.
The other failed check is because you need to go mod tidy the go.mod file.

hoshsadiq · 2025-04-14T20:51:34Z

I've rebased. I'm unsure about the go mod tidy. It doesn't return anything, but my ide does fail on a bunch of things for k8s.io that I have to add as replace directives:

// Add replace directives for k8s.io modules with version issues
replace (
        k8s.io/cloud-provider => k8s.io/cloud-provider v0.32.2
        k8s.io/cluster-bootstrap => k8s.io/cluster-bootstrap v0.32.2
        k8s.io/cri-api => k8s.io/cri-api v0.32.2
        k8s.io/cri-client => k8s.io/cri-client v0.0.0-20240429231629-220bfaa459cb
        k8s.io/csi-translation-lib => k8s.io/csi-translation-lib v0.32.2
        k8s.io/dynamic-resource-allocation => k8s.io/dynamic-resource-allocation v0.32.2
        k8s.io/endpointslice => k8s.io/endpointslice v0.32.2
        k8s.io/externaljwt => k8s.io/externaljwt v0.32.2
        k8s.io/kube-controller-manager => k8s.io/kube-controller-manager v0.32.2
        k8s.io/kube-proxy => k8s.io/kube-proxy v0.32.2
        k8s.io/kube-scheduler => k8s.io/kube-scheduler v0.32.2
        k8s.io/kubelet => k8s.io/kubelet v0.32.2
        k8s.io/mount-utils => k8s.io/mount-utils v0.32.2
        k8s.io/pod-security-admission => k8s.io/pod-security-admission v0.32.2
        k8s.io/sample-apiserver => k8s.io/sample-apiserver v0.32.2
)

yet none of them are actually needed according to go mod

$ go mod why k8s.io/cloud-provider k8s.io/cluster-bootstrap k8s.io/cri-api k8s.io/cri-client k8s.io/csi-translation-lib k8s.io/dynamic-resource-allocation k8s.io/endpointslice k8s.io/externaljwt k8s.io/kube-controller-manager k8s.io/kube-proxy k8s.io/kube-scheduler k8s.io/kubelet k8s.io/mount-utils k8s.io/pod-security-admission k8s.io/sample-apiserver
go: downloading k8s.io/cri-client v0.0.0-20240429231629-220bfaa459cb
# k8s.io/cloud-provider
(main module does not need package k8s.io/cloud-provider)

# k8s.io/cluster-bootstrap
(main module does not need package k8s.io/cluster-bootstrap)

# k8s.io/cri-api
(main module does not need package k8s.io/cri-api)

# k8s.io/cri-client
(main module does not need package k8s.io/cri-client)

# k8s.io/csi-translation-lib
(main module does not need package k8s.io/csi-translation-lib)

# k8s.io/dynamic-resource-allocation
(main module does not need package k8s.io/dynamic-resource-allocation)

# k8s.io/endpointslice
(main module does not need package k8s.io/endpointslice)

# k8s.io/externaljwt
(main module does not need package k8s.io/externaljwt)

# k8s.io/kube-controller-manager
(main module does not need package k8s.io/kube-controller-manager)

# k8s.io/kube-proxy
(main module does not need package k8s.io/kube-proxy)

# k8s.io/kube-scheduler
(main module does not need package k8s.io/kube-scheduler)

# k8s.io/kubelet
(main module does not need package k8s.io/kubelet)

# k8s.io/mount-utils
(main module does not need package k8s.io/mount-utils)

# k8s.io/pod-security-admission
(main module does not need package k8s.io/pod-security-admission)

# k8s.io/sample-apiserver
(main module does not need package k8s.io/sample-apiserver)

hnnsgstfssn · 2025-04-14T20:57:56Z

The other failed check is because you need to go mod tidy the go.mod file.

Just want to note that we have an open support ticket with Github on this failed check on Dependabot pull requests since it seems to be a bug with Dependabot.

hnnsgstfssn

Could we also update ./.github/PULL_REQUEST_TEMPLATE.md and ./.github/ISSUE_TEMPLATE/BUG_REPORT.md?

Oded-B · 2025-04-15T10:54:14Z

The other failed check is because you need to go mod tidy the go.mod file.

Just want to note that we have an open support ticket with Github on this failed check on Dependabot pull requests since it seems to be a bug with Dependabot.

Are you sure it's related to Dependabot? This PR changes the ordering of dependeices in go.mod , go mod tidy is specific about ordering so the check fails.

And these are the exact lines go mod want to change:

I think it's just a matter of moving the these line back to their original postions, or am I missing something?

hoshsadiq · 2025-04-20T19:39:22Z

Sorry, for some reason I don't get notified on this thread. I've updated those two extra files (unsure how I missed it since I used sed 🤔 ). Anyway, yeah the go mod changes are not dependabot related, but I'm unsure what I can do here besides manually make changes to go.mod. Running go mod tidy doesn't make that same change.

hoshsadiq · 2025-04-20T19:41:47Z

Oh, interesting, go 1.24.2 and 1.24.1 seem to produce a different go.mod. Should be fixed now.

Oded-B · 2025-04-22T10:38:21Z

Ah, more linting issues :/
This time its the dependency ordeting in the golang files themselves 🤦

Usually the IDE takes care of this but in cases when you Sed across multiple files its usually faster to do it via the CLI:

go install github.com/daixiang0/gci@latest #if not installed already
gci write .

hoshsadiq · 2025-04-28T10:08:29Z

Also fixed now!

sunchill06 · 2025-04-29T09:11:42Z

@Oded-B could you review this again? This is blocking some Dependabot PRs.

Oded-B · 2025-04-29T11:26:48Z

I approved, but it's waiting for @hnnsgstfssn (i think).
I'm not near a computer but from my POV we should merge it, now, with admin override if needs be

hnnsgstfssn

All good from me- apologies but have been on annual leave this past week.

hnnsgstfssn · 2025-04-29T16:45:34Z

@hoshsadiq there is a requirement on signed commits. Would you mind setting that up and force push signed commits? Here's the Github docs on how to do it.

I'll add a PR to mention this in the contributions doc.

There is a requirement for signed commits before merging a pull request and this is not documented in the contributing document. This came up while trying to get [1] merged. [1] #106

hoshsadiq · 2025-04-30T15:34:03Z

Ohh, fun, I thought I had set it up. Will do soon

There is a requirement for signed commits before merging a pull request and this is not documented in the contributing document. This came up while trying to get [1] merged. [1] #106

hoshsadiq · 2025-05-13T23:35:54Z

Sorry it took so long, but force pushed a verified commit now.

Oded-B · 2025-05-14T15:16:08Z

Merged with the failed lint run.
I'll fix that later

hoshsadiq requested a review from a team as a code owner April 11, 2025 11:27

hoshsadiq requested review from hnnsgstfssn, Freyert, ashvarts, yzdann and sunchill06 April 11, 2025 11:27

Oded-B mentioned this pull request Apr 14, 2025

Inject default values to container image name to avoid CI failure in from-fork PRs #108

Merged

13 tasks

hoshsadiq force-pushed the fix-import branch from 228cac8 to 10da7ab Compare April 14, 2025 20:44

hnnsgstfssn reviewed Apr 14, 2025

View reviewed changes

Oded-B approved these changes Apr 29, 2025

View reviewed changes

Oded-B enabled auto-merge (squash) April 29, 2025 11:24

Oded-B disabled auto-merge April 29, 2025 11:24

hnnsgstfssn approved these changes Apr 29, 2025

View reviewed changes

hoshsadiq force-pushed the fix-import branch from c81f12d to b479d67 Compare May 13, 2025 23:26

hoshsadiq force-pushed the fix-import branch from b479d67 to 2cc71e5 Compare May 13, 2025 23:30

Fix import from old org

3c37d38

hoshsadiq force-pushed the fix-import branch from 2cc71e5 to 3c37d38 Compare May 13, 2025 23:35

Merge branch 'main' into fix-import

67ca926

Oded-B merged commit 6a5f8dd into commercetools:main May 14, 2025
7 of 8 checks passed

hoshsadiq deleted the fix-import branch May 14, 2025 18:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix import from old org #106

Fix import from old org #106

hoshsadiq commented Apr 11, 2025 •

edited

Loading

Oded-B commented Apr 11, 2025

Oded-B commented Apr 11, 2025

Oded-B commented Apr 14, 2025

hoshsadiq commented Apr 14, 2025

hnnsgstfssn commented Apr 14, 2025

hnnsgstfssn left a comment

Oded-B commented Apr 15, 2025

hoshsadiq commented Apr 20, 2025

hoshsadiq commented Apr 20, 2025

Oded-B commented Apr 22, 2025

hoshsadiq commented Apr 28, 2025

sunchill06 commented Apr 29, 2025

Oded-B commented Apr 29, 2025

hnnsgstfssn left a comment

hnnsgstfssn commented Apr 29, 2025

hoshsadiq commented Apr 30, 2025

hoshsadiq commented May 13, 2025

Oded-B commented May 14, 2025

Fix import from old org #106

Fix import from old org #106

Conversation

hoshsadiq commented Apr 11, 2025 • edited Loading

Description

Type of Change

Checklist

Oded-B commented Apr 11, 2025

Oded-B commented Apr 11, 2025

Oded-B commented Apr 14, 2025

hoshsadiq commented Apr 14, 2025

hnnsgstfssn commented Apr 14, 2025

hnnsgstfssn left a comment

Choose a reason for hiding this comment

Oded-B commented Apr 15, 2025

hoshsadiq commented Apr 20, 2025

hoshsadiq commented Apr 20, 2025

Oded-B commented Apr 22, 2025

hoshsadiq commented Apr 28, 2025

sunchill06 commented Apr 29, 2025

Oded-B commented Apr 29, 2025

hnnsgstfssn left a comment

Choose a reason for hiding this comment

hnnsgstfssn commented Apr 29, 2025

hoshsadiq commented Apr 30, 2025

hoshsadiq commented May 13, 2025

Oded-B commented May 14, 2025

hoshsadiq commented Apr 11, 2025 •

edited

Loading