Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should we remove the Flavor from source to avoid the confusion? #96

Closed
zhangtbj opened this issue Apr 8, 2020 · 4 comments
Closed

Should we remove the Flavor from source to avoid the confusion? #96

zhangtbj opened this issue Apr 8, 2020 · 4 comments

Comments

@zhangtbj
Copy link
Contributor

zhangtbj commented Apr 8, 2020

Hi @sbose78 ,

We added a GitLab test in this PR: Add env variables for the e2e tests (#87)

And in the test, we just need to input the GitLab URL like Github in the spec.source.url:
https://github.com/redhat-developer/build/blob/master/test/data/build_buildah_cr_private_gitlab.yaml#L8

So I thought all repo should follow a similar standard that we don't need to do any additional render work.

But now, we still have a Flavor parameter in source spec:
https://github.com/redhat-developer/build/blob/master/pkg/apis/build/v1alpha1/gitsource.go#L34

I think if all repo can use the same style, we should remove it to avoid the confusion now.

@sbose78
Copy link
Member

sbose78 commented Apr 8, 2020

The reason for adding the flavour was to support git provider based webhook-related behaviour in the future.

At the moment, I don't have design in mind to implement it. I'm okay to remove it now and bring it back later on when we have a design in place which consumes this.

@zhangtbj
Copy link
Contributor Author

zhangtbj commented Apr 8, 2020

Agree.

em... interesting.

For the webhook or trigger functions, I think we can have more discussion about how to do it better in the future.

@qu1queee
Copy link
Contributor

@zhangtbj I think the Flavor field is not used in our logic, it seems the answer is lets keep the field for now while it might be useful in the future. If this is the case, can we close this issue?

@zhangtbj
Copy link
Contributor Author

Sure thing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants