Port over odo parser

[yangcao77]

What does this PR do?

This PR moves the parser pkg from Odo repo to devfile/parser, and integrates with latest devfile go structs defined in devfile/api

What issues does this PR fix or reference?

Fixes devfile/api#183

Is your PR tested? Consider putting some instruction how to test your changes

Tested by running the unit tests within this PR.

[amisevsk]

In general I think it's a bad idea to have no-logic getters on exported fields (e.g. return d.Parent) unless we can justify more complicated logic in the future.

[yangcao77]

@amisevsk they reason why we have those Get() functions is because information like d.component is not exposed in devfileObj which consumers will get after parsing. They can only use interface defined in devfileData to access the struct.
This is how the initial devfile parser was implemented, and is how odo currently using and implemented.

[wtam2018]

Should FakeContext be move to testingutil package?

[yangcao77]

all fields inDevfileCtx are defined as internal, and cannot be accessible outside of the context package

[wtam2018]

You are right. Let's move FakeContext() to xxx_test.go so that it is not visible outside of tests. Also, we should consider not exporting this function.

[yangcao77]

I agree we should rename the file to avoid confusion.
The function should be kept as exported, because consumers(like odo) would need that function for their testing purpose.

[kadel]

I don't thing that this is a good approach.

I was under assumption that we have agreement on evolving and doing all necessary changes to parser in odo repo first, and then once we have independent parser package in odo we extract it from odo and put it here.
If we start evolving parser package like this in a separate repo odo and this parser package will drift apart and we will never be able to switch to this.

The first step was supposed to be in a the opposite direction. If there are additional things in devfile/parser that are not in odo it should be added into odo parser package.

[wtam2018]

When will odo parser finish with evolving?

[kadel]

I don't thing that this is a good approach.

I was under assumption that we have agreement on evolving and doing all necessary changes to parser in odo repo first, and then once we have independent parser package in odo we extract it from odo and put it here.
If we start evolving parser package like this in a separate repo odo and this parser package will drift apart and we will never be able to switch to this.

The first step was supposed to be in a the opposite direction. If there are additional things in devfile/parser that are not in odo it should be added into odo parser package.

I haven't seen this issue on odo side before redhat-developer/odo#4117 (comment)
It addresses all my concerns. I just wanted to make sure that we don't have the same logic in odo and in devfile/parser.
The issue talks about using devfile/parser in odo. We just have to make sure that it happens right after this PR is merged.

[johnmcollier]

Should we be vendoring the go module dependencies here? Might be a good idea

[amisevsk]

IMO if we're going to start vendoring dependencies in this repo, it should be done in a separate PR.

[yangcao77]

@amisevsk I'm fine with either way. I will modify the commit history if we want to remove the vendor folder. I don't want the git history to be too big.
Is that the only concern from you?

@adisky And additional concerns?

I would like to get approval from you two before merging this PR.
Let me know if you found difficulty reviewing the PR due to the vendor folder. I will remove it if that's the case.

[adisky]

I don't want the git history to be too big.

@yangcao77 To keep the history clean you can squash all the commits before merging

@adisky And additional concerns?

I would like to get approval from you two before merging this PR.
Let me know if you found difficulty reviewing the PR due to the vendor folder. I will remove it if that's the case.

For Porting purpose from odo to here, It is good to go from my side but more work needed to make it consumable as library
Logged two issues in devfile/api
devfile/api#194
devfile/api#193

[amisevsk]

If we're squashing this commit before merging then removing the vendoring is essential -- otherwise the commit that ends up in the history is completely useless from a history point of view. I have no problem with force-pushes to PR branches personally.

Vendor vs not vendor is a topic worth discussing and not something we should slide into a PR about something generally separate.

[yangcao77]

I do not want to squash all commit into one single commit, that would lose all commit history inside this PR.
Instead, I have removed the commits related to vendoring from commit history of the PR.
Let me know if you have any other concerns. @amisevsk

[yangcao77]

I'm going to merge this PR if no additional concerns have been raised end of Tuesday(EDT). We need this PR to proceed our next step. Thanks.

[maysunfaisal]

All the review comments were addressed for the PR

/lgtm