-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Remove vendor folder from the repository #18655
Remove vendor folder from the repository #18655
Conversation
Pinging @elastic/integrations-services (Team:Services) |
Is there any recovery plan if the original source code of a dependency is removed? I mean, the author decides to remove the entire repository? Isn't it a too big risk? |
💔 Build FailedExpand to view the summary
Build stats
Steps errorsExpand to view the steps failures
Log outputExpand to view the last 100 lines of log output
|
@mtojek It is indeed a big risk. That is why we are going to use Artifactory to mirror these packages. Adopting Artifactory is the second task from the todo list. I am rewording it to avoid confusion around the reason why we need it. |
729f91d
to
445b035
Compare
68b026a
to
6c96cd4
Compare
💔 Build FailedExpand to view the summary
Build stats
Steps errorsExpand to view the steps failures
Log outputExpand to view the last 100 lines of log output
|
6c96cd4
to
484a407
Compare
484a407
to
b6475c4
Compare
jenkins run tests |
f2c1438
to
3c11eb0
Compare
1b8983b
to
612dbdf
Compare
0ec883c
to
132362b
Compare
Our policy requires us only to add the contents of the NOTICE files of each third party dependency. We do not have to add any additional stuff. The only reason we might do is to have information about our dependencies at that point in time. |
I've rebased the branch and addressed the docs issues. |
👍 Thanks for clarification. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR is quite big, which makes it a little difficult to see all changes.
Looks like vendor
has been removed correctly in every go command and NOTICE file is generated. All in all LGTM
Most important are NOTICE file and green CI. Please make sure to have a green CI before merging.
Please have this reviewed by at least another Person.
run beats-ci/package |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, I have added some small comments on mage and make targets.
d520585
to
58c06ee
Compare
run beats-ci/package |
/packaging |
58c06ee
to
7bff601
Compare
/packaging |
7bff601
to
921859f
Compare
/packaging |
jenkins run tests |
jenkins run tests please |
/packaging |
To update or add a dependency use the standard tools provided by Golang. (cherry picked from commit 5426df0)
PR elastic#18655 removed the vendor folder.
To update or add a dependency use the standard tools provided by Golang.
What does this PR do?
This PR removes the vendor folder from the repository.
go.elastic.co/go-licence-detector
to generate NOTICE file- [ ] use package proxy/mirror when running CI jobs and also to use it locally if neededthe generated Beats use vendor folder, but users are free to move away from it
By removing the folder we become vulnerable to new dangers:
To combat these we are adopting Artifactory and mirroring these modules to our own infra. Before we merge this PR we need to make sure we are prepared to mitigate these issues. So a working Artifactory mirror is a must-have beforehand.We decided to merge this without Artifactory. APM team had no issues with building their binaries consistently without an own package mirror, so we are positive this decision won't have a big impact on our builds.
To update dependencies, use the standard tools provided by Golang:
Why is it important?
We need to remove dependencies, as we are not allowed to vendor every dependency we use.
Checklist
CHANGELOG.next.asciidoc
orCHANGELOG-developer.next.asciidoc
.