devfile / api

Kube-native API for cloud development workspaces specification
Apache License 2.0
237 stars 58 forks source link

add go mod to test to enable import from library repo #356

Closed mmulholla closed 3 years ago

mmulholla commented 3 years ago

Add go.mod for the test directory to enable library repository tests to import from it.

openshift-ci-robot commented 3 years ago

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: mmulholla To complete the pull request process, please assign after the PR has been reviewed. You can assign the PR to them by writing /assign in a comment when ready.

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

Needs approval from an approver in each of these files: - **[OWNERS](https://github.com/devfile/api/blob/master/OWNERS)** Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
mmulholla commented 3 years ago

@amisevsk the library repository needs to import from test/v200/utils/common as that is shared code with the library repository tests

amisevsk commented 3 years ago

the library repository needs to import from test/v200/utils/common as that is shared code with the library repository tests

Can't the library just import it from github.com/devfile/api/v2/test/v200/utils/common?

mmulholla commented 3 years ago

the library repository needs to import from test/v200/utils/common as that is shared code with the library repository tests

Can't the library just import it from github.com/devfile/api/v2/test/v200/utils/common?

Not unless the tests are moved under the pkg directory. What I added is similar to the /api/generator directory Here is where you can see what is currently avallable for import: https://pkg.go.dev/github.com/devfile/api

mmulholla commented 3 years ago

not sure why we're vendoring the repo, is there a reason?

Just copied what other directories with go.mod do. I also suspect the build will fail if it is not there.

mmulholla commented 3 years ago

I am held up by this, please approve, if you can't approve we better go back and revisit the generator directory.

mmulholla commented 3 years ago

@maysunfaisal the vendor directory has been removed.

amisevsk commented 3 years ago

Creating a new project with the go mod file

module test

go 1.14

require github.com/devfile/api/v2 v2.0.0-20210223145532-81859eaef987 // the current main commit

allows me to e.g. import commonUtils no problem:

package main

import (
    commonUtils "github.com/devfile/api/v2/test/v200/utils/common"
)

func main() {
    commonUtils.GetDevFileName()
}

No idea why this isn't importing automatically, I suspect it's a stale goproxy. because the highest tagged release is https://github.com/devfile/api/releases/tag/v2.0.0, from Jan 18th. Making a submodule will have the same problem until a new tag is created or consumers manually set the version via commit (which is what we do in the DevWorkspace Operator)

amisevsk commented 3 years ago

Playing with it a bit more, running go get -u github.com/devfile/api/v2@81859eaef987 in the library repo will fix the problem.

mmulholla commented 3 years ago

Playing with it a bit more, running go get -u github.com/devfile/api/v2@81859eaef987 in the library repo will fix the problem.

Thx, I switched to a new command window and that seem to fix the library issue importing validation too.

mmulholla commented 3 years ago

Creating a new project with the go mod file

module test

go 1.14

require github.com/devfile/api/v2 v2.0.0-20210223145532-81859eaef987 // the current main commit

allows me to e.g. import commonUtils no problem:

package main

import (
  commonUtils "github.com/devfile/api/v2/test/v200/utils/common"
)

func main() {
  commonUtils.GetDevFileName()
}

No idea why this isn't importing automatically, I suspect it's ~a stale goproxy.~ because the highest tagged release is https://github.com/devfile/api/releases/tag/v2.0.0, from Jan 18th. Making a submodule will have the same problem until a new tag is created or consumers manually set the version via commit (which is what we do in the DevWorkspace Operator)

OK, I have changed the module in go.sum to be just "test" and locally import from "api/v2/test" and everything appears to work for the tests in the api repository. Not sure I can test import into the library repository until this is merged.

mmulholla commented 3 years ago

After discussions with Angel (thank you) it appears that this PR is redundant, the api test directory does not need to create a module for the library repository to import it. Testing now, but closing this PR.