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

pre-requirement to make istio/envoy ci test happen in CI. #3682

Closed
wants to merge 3 commits into from
Closed

pre-requirement to make istio/envoy ci test happen in CI. #3682

wants to merge 3 commits into from

Conversation

innerpeacez
Copy link
Member

Please answer these questions before submitting pull request

  • Why submit this pull request?

  • Bug fix

  • New feature provided

  • Improve performance

  • Related issues


Bug fix

  • Bug description.

  • How to fix?


New feature or improvement

  • Describe the details and related test reports.

hanahmily
hanahmily previously approved these changes Oct 22, 2019
Copy link
Contributor

@hanahmily hanahmily left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@wu-sheng wu-sheng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lack of Apache 2.0 header in many files.

@wu-sheng wu-sheng added the documentation Provide documents related issue or pull request only. label Oct 22, 2019
@wu-sheng wu-sheng added this to the 6.5.0 milestone Oct 22, 2019
hosts:
- skywalking.domain.com
```
### Envoy ALS
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wu-sheng
Copy link
Member

The old k8s doc is updated after our official release, how this works? Is it working with source code? If so, where is the process to prepare the docker image?

I am feeling the document is missing somehow.

@wu-sheng
Copy link
Member

@hanahmily And with this is going to be merged, what is the agenda for skywalking-kubernetes repo? Is that repo being abandoned? Or?

@innerpeacez
Copy link
Member Author

#3682 (comment)

#3682 (comment)

resolved @wu-sheng

@hanahmily
Copy link
Contributor

hanahmily commented Oct 22, 2019 via email

@wu-sheng
Copy link
Member

I mean the process, before merge this, I think we at least should have suitable process for release and heln update. Could we add the process to release doc?

@wu-sheng
Copy link
Member

Otherwise, I have concerns. Release begin maybe in two weeks later. So, we need a clear purpose of this.

@kezhenxu94
Copy link
Member

@innerpeacez can you re-title this PR? It's too vague, note that it will become the commit message in our codebase commit history

@innerpeacez innerpeacez changed the title for #3669 pre-requirement to make istio/envoy ci test happen in CI. Oct 22, 2019
@hanahmily
Copy link
Contributor

hanahmily commented Oct 23, 2019 via email

@wu-sheng
Copy link
Member

Do you two need to collaborate in upstream code base? Or just a fork repo is good enough?
Your process is clear to me. But merge into master and release of 6.5, 6.6 are still my concern.

@hanahmily
Copy link
Contributor

hanahmily commented Oct 23, 2019 via email

@wu-sheng
Copy link
Member

But it will still misguide user. I want to offer a branch, istio-ci, which could use jenkins and our infra too, and don't need to be concernd about release issue.
Make sense?

@hanahmily
Copy link
Contributor

hanahmily commented Oct 23, 2019 via email

@innerpeacez
Copy link
Member Author

Does the branch name need to be discussed? install/kubernetes or istio-ci ? @wu-sheng @hanahmily

@wu-sheng
Copy link
Member

We should use istio-ci. As this is a primary goal as step1.

@wu-sheng
Copy link
Member

Closing this. Branch istio-ci added, please submit the PR to there. https://github.com/apache/skywalking/tree/istio-ci, and tag the issue as ci-setting in milestone 6.6.

@wu-sheng wu-sheng closed this Oct 23, 2019
@hanahmily
Copy link
Contributor

hanahmily commented Oct 23, 2019 via email

@innerpeacez
Copy link
Member Author

Ok, I will submit it to the istio-ci branch soon. thanks a lot. @wu-sheng @hanahmily

@wu-sheng wu-sheng added the wontfix This will not be worked on label Oct 24, 2019
@innerpeacez innerpeacez deleted the test-envoy branch October 24, 2019 08:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Provide documents related issue or pull request only. wontfix This will not be worked on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants