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

Adding vendor folder to gitignore file #1085

Merged
merged 3 commits into from
May 3, 2023

Conversation

raj-prince
Copy link
Collaborator

@raj-prince raj-prince commented May 3, 2023

Description

Removing the vendor directory from the gcsfuse codebase. Change contains:
(a) Added vendor directory in the .gitignore file.
(b) Deleted vendor directory using git rm --cached -r vendor command.

Check Required

  1. Building the codebase: We can build the codebase using the same command. There is not change at all. So, when we build the first time it fetches all the dependencies in the "GOCACHE" (go env) folder, which takes around 30 seconds. After that it uses the same cache to build which takes around 3-4 seconds. Hence, we don't require any change in any of the scripts.
  2. Truly build reproducible: By default go module contains go.mod and go.sum file to maintain an immutable database, which makes the build process truly build reproducible. Ref - https://go.dev/blog/module-mirror-launch
  3. Test code locally in changing the dependency code: You can use https://go.dev/ref/mod#go-mod-file-replace to test while changing in the dependency code.
  4. I'll raise another PR to setup cache in the CI/CD workflow.

Link to the issue in case of a bug fix.

Testing details

  1. Manual - (a) Manually verified all the basic file system operations. (b) Verified the build process by cleaning the go-cache go clean -cache.
  2. Unit tests - Automation
  3. Integration tests - Automation

@raj-prince raj-prince self-assigned this May 3, 2023
@raj-prince raj-prince added the execute-perf-test Execute performance test in PR label May 3, 2023
Copy link
Collaborator

@sethiay sethiay left a comment

Choose a reason for hiding this comment

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

Looks good.

  1. I think the perf tests are not executed even with execute-perf-test label. I hope that's expected.
  2. If it takes 30-40 seconds to build without vendor directory, then i think it's fine to not add cache in CI/CD pipeline ? Let me know if I am missing something.

[raj-prince] Yeah, we can skip for now. In future, if it requires then we can add.

@Tulsishah
Copy link
Collaborator

Tulsishah commented May 3, 2023

Looks good.

  1. I think the perf tests are not executed even with execute-perf-test label. I hope that's expected.
  2. If it takes 30-40 seconds to build without vendor directory, then i think it's fine to not add cache in CI/CD pipeline ? Let me know if I am missing something.

Perf tests are not executed because @raj-prince attached a label and hasn't pushed any commit after that. You can run it through test fusion directly, also.

[raj-prince] Added an empty commit to run the perf test.

@raj-prince
Copy link
Collaborator Author

raj-prince commented May 3, 2023

image

Performance numbers are almost same. Merging this.

@raj-prince raj-prince merged commit f67107d into GoogleCloudPlatform:master May 3, 2023
ashmeenkaur added a commit that referenced this pull request May 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
execute-perf-test Execute performance test in PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants