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.

Sign up for GitHub

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

Jump to bottom

Removed vendor directory #46

Merged
merged 1 commit into from
May 31, 2021
Merged

Removed vendor directory #46

merged 1 commit into from
May 31, 2021

Conversation

arnaud-tincelin
Copy link
Collaborator

Removed the vendor directory and fixed build errors due to new package versions

@Tatsinnit Tatsinnit requested a review from JunSun17 April 24, 2021 19:31
@JunSun17
Copy link
Collaborator

JunSun17 commented Apr 26, 2021 •
edited
Loading

@arnaud-tincelin Why do you remove the vendor directory? It looks will impact dependency and also some license files.

@arnaud-tincelin
Copy link
Collaborator Author

Because I am relying on go modules. Dependencies list & versions can be found in the go.mod file.

@guilhem
Copy link
Contributor

guilhem commented May 4, 2021

Current /vendor is totally outdated (last commit 2y ago).
For information, current /vendor should look like this: arnaud-tincelin@e5b8f75 (199 files change with 221,385 additions and 3,712 deletions).

It shows us that this folder wasn't used and should be a problem for anyone relying on it.

@JunSun17
Copy link
Collaborator

JunSun17 commented May 4, 2021

@Tatsinnit This looks good to me, can you hep to verify the new vendoring works as expected? Thanks!

@Tatsinnit
Copy link
Member

Sound a good and thanks @JunSun17 , indeed, @arnaud-tincelin and I have discussed this and this will only be merged once we have tested that the local image worked well. Thanks 🙏

sophsoph321 pushed a commit to sophsoph321/aks-periscope that referenced this pull request May 7, 2021
# This is a combination of 127 commits.
2854366 
# This is the 1st commit message:

Revert "introduce arcmode"

This reverts commit 5f4fed4.

# This is the commit message #2:

remove secrets

# This is the commit message Azure#3:

add print statement

# This is the commit message Azure#4:

update print statement

# This is the commit message Azure#5:

committed

# This is the commit message Azure#6:

committed

# This is the commit message Azure#7:

committed

# This is the commit message Azure#8:

committed

# This is the commit message Azure#9:

remove print statements

# This is the commit message Azure#10:

add helm collector

# This is the commit message Azure#11:

change helm command

# This is the commit message Azure#12:

add helm 3 installation

# This is the commit message Azure#13:

add curl command installation

# This is the commit message Azure#14:

change helm command

# This is the commit message Azure#15:

remove helm history

# This is the commit message Azure#16:

debug helm history

# This is the commit message Azure#17:

add repo command

# This is the commit message Azure#18:

change stable repo name

# This is the commit message Azure#19:

add write to file

# This is the commit message Azure#20:

add kured

# This is the commit message Azure#21:

change

# This is the commit message Azure#22:

changes

# This is the commit message Azure#23:

add default namespace

# This is the commit message Azure#24:

change

# This is the commit message Azure#25:

add integration test

# This is the commit message Azure#26:

changes

# This is the commit message Azure#27:

add helm test

# This is the commit message Azure#28:

change print statement to error

# This is the commit message Azure#29:

change

# This is the commit message Azure#30:

more changes

# This is the commit message Azure#31:

add go installation

# This is the commit message Azure#32:

fix unit test

# This is the commit message Azure#33:

iptables to Helm

# This is the commit message Azure#34:

add custom resource collector

# This is the commit message Azure#35:

add new exporter, diagnoser, collector

# This is the commit message Azure#36:

comment unused variables

# This is the commit message Azure#37:

debug exporter

# This is the commit message Azure#38:

filenames

# This is the commit message Azure#39:

test zip function

# This is the commit message Azure#40:

list files

# This is the commit message Azure#41:

fmt to log

# This is the commit message Azure#42:

delete lines

# This is the commit message Azure#43:

changed

# This is the commit message Azure#44:

get current directory

# This is the commit message Azure#45:

remove some print statements

# This is the commit message Azure#46:

test zip

# This is the commit message Azure#47:

changes

# This is the commit message Azure#48:

add windir check

# This is the commit message Azure#49:

minor fix

# This is the commit message Azure#50:

get hostname

# This is the commit message Azure#51:

add expose in dockerfile

# This is the commit message Azure#52:

add exec collector

# This is the commit message Azure#53:

mitigate exit code 126

# This is the commit message Azure#54:

change curl url from example.com to dp endpoint

# This is the commit message Azure#55:

changes

# This is the commit message Azure#56:

uncomment exec

# This is the commit message Azure#57:

add new diagnoser

# This is the commit message Azure#58:

debugging

# This is the commit message Azure#59:

debug

# This is the commit message Azure#60:

debugging

# This is the commit message Azure#61:

remove print statements

# This is the commit message Azure#62:

remove print

# This is the commit message Azure#63:

add back crd print statement

# This is the commit message Azure#64:

change

# This is the commit message Azure#65:

change

# This is the commit message Azure#66:

update dataPoint name

# This is the commit message Azure#67:

modify forloop

# This is the commit message Azure#68:

add filename to datapoint

# This is the commit message Azure#69:

add back log prints

# This is the commit message Azure#70:

test

# This is the commit message Azure#71:

add fields to diagnostic signal

# This is the commit message Azure#72:

add config content to diagnoser

# This is the commit message Azure#73:

change format from yaml to json

# This is the commit message Azure#74:

add parameters for kubeobject config map

# This is the commit message Azure#75:

Revert "introduce arcmode"

This reverts commit 5f4fed4.

# This is the commit message Azure#76:

fix helm collector style

# This is the commit message Azure#77:

revert changes that test arc customizations

# This is the commit message Azure#78:

fix merge conflicts

# This is the commit message Azure#79:

fix merge conflicts

# This is the commit message Azure#80:

Revert "Add v0.3 acr image for Private cluster fix. (Azure#22)"

This reverts commit 49dd302.

# This is the commit message Azure#81:

fix merge conflicts

# This is the commit message Azure#82:

fix merge conflicts

# This is the commit message Azure#83:

add print statement

# This is the commit message Azure#84:

update print statement

# This is the commit message Azure#85:

committed

# This is the commit message Azure#86:

committed

# This is the commit message Azure#87:

committed

# This is the commit message Azure#88:

committed

# This is the commit message Azure#89:

remove print statements

# This is the commit message Azure#90:

fix merge conflicts

# This is the commit message Azure#91:

fix merge conflicts

# This is the commit message Azure#92:

fix merge conflicts

# This is the commit message Azure#93:

add repo command

# This is the commit message Azure#94:

change stable repo name

# This is the commit message Azure#95:

add write to file

# This is the commit message Azure#96:

add kured

# This is the commit message Azure#97:

change

# This is the commit message Azure#98:

changes

# This is the commit message Azure#99:

add default namespace

# This is the commit message Azure#100:

change

# This is the commit message Azure#101:

add integration test

# This is the commit message Azure#102:

changes

# This is the commit message Azure#103:

add helm test

# This is the commit message Azure#104:

change print statement to error

# This is the commit message Azure#105:

change

# This is the commit message Azure#106:

more changes

# This is the commit message Azure#107:

add go installation

# This is the commit message Azure#108:

fix unit test

# This is the commit message Azure#109:

add custom resource collector

# This is the commit message Azure#110:

fix merge conflicts

# This is the commit message Azure#111:

comment unused variables

# This is the commit message Azure#112:

debug exporter

# This is the commit message Azure#113:

filenames

# This is the commit message Azure#114:

test zip function

# This is the commit message Azure#115:

list files

# This is the commit message Azure#116:

fmt to log

# This is the commit message Azure#117:

delete lines

# This is the commit message Azure#118:

changed

# This is the commit message Azure#119:

get current directory

# This is the commit message Azure#120:

remove some print statements

# This is the commit message Azure#121:

test zip

# This is the commit message Azure#122:

changes

# This is the commit message Azure#123:

add windir check

# This is the commit message Azure#124:

minor fix

# This is the commit message Azure#125:

get hostname

# This is the commit message Azure#126:

add expose in dockerfile

# This is the commit message Azure#127:

add exec collector
@arnaud-tincelin
Copy link
Collaborator Author

Update: in order to simplify this PR the package versions are not updated anymore (no more code changes).
Performed actions in this PR:

  • removed vendor directory
  • go mod tidy

@Tatsinnit
Copy link
Member

Cool, @arnaud-tincelin, please let me know if you have a test image with these changes and I can also give a good test for existing changes and impact on current functionality. Thank you.

Tatsinnit
Tatsinnit requested changes May 11, 2021
View reviewed changes
Copy link
Member

@Tatsinnit Tatsinnit left a comment •
edited
Loading

Choose a reason for hiding this comment

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

💡 Added minor comment because when I was trying tp publish my local image it error'd out due to -mod=vendor in the line 5 for build/dockerfile

  • I will test it today and you can do the same so that we have made sure existing functionality all behaved the same way.
    • Update: I tested with vscode which intern using deployment with my local docker image it seems to be working as usual, will be worth if you double check as well.

Rest looks good, thank you, I believe only change we need it to change the Dockerfile line 5.

Thank you so much. 🙏

go.sum Show resolved Hide resolved
This was referenced May 12, 2021
Closed
Closed
@arnaud-tincelin arnaud-tincelin force-pushed the golang-modules branch 4 times, most recently from 702094e to 49a2651 Compare May 26, 2021 09:47
Tatsinnit
Tatsinnit requested changes May 26, 2021
View reviewed changes
Copy link
Member

@Tatsinnit Tatsinnit left a comment

Choose a reason for hiding this comment

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

💡 I would do a little ping to @JunSun17 for his final eyes, beside 2 key points rest looks good.

I will test it at my end as well. Thanks.

README.md Outdated Show resolved Hide resolved
go.mod Show resolved Hide resolved
Remove vendor directory
355a250 
Updated Dockerfile
Removed -mod=vendor option from dockerfile & cleanup
@Tatsinnit Tatsinnit merged commit 8bd57bb into Azure:master May 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Tatsinnit Tatsinnit Tatsinnit approved these changes

@JunSun17 JunSun17 Awaiting requested review from JunSun17

Assignees

@arnaud-tincelin arnaud-tincelin

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@arnaud-tincelin @JunSun17 @guilhem @Tatsinnit