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

Add the vendor directory to .gitignore #786

Merged
merged 2 commits into from
Jun 24, 2024

Conversation

renormalize
Copy link
Member

@renormalize renormalize commented Apr 19, 2024

How to categorize this PR?
/area dev-productivity
/kind enhancement

What this PR does / why we need it:
#748 removed the vendor directory from etcd-druid and makes the project rely on the module cache set up by the go tool for finding dependencies.

However, there are use cases where for better developer experience, it would make sense to have the dependencies reside in the same project folder as etcd-druid, so that tokens can be searched for, using project wide search in any text editor. Results can be fetched from the dependencies, without having to rely on the LSP. For example, this would make looking at all usages of a constant that is defined by a dependency easier.

The developer can simply run go mod vendor without it affecting the modified build process which doesn't use vendor, as defined in #748.

However, to not break the current build process which doesn't use the vendor directory anymore, rename the vendor directory to anything else while running make run or make docker-build (mv vendor .vendor for example).

Special notes for your reviewer:
/hold till #777 merges.

NONE

* gardener#748 removed the `vendor` directory from `etcd-druid` and makes the
  project rely on the module cache set up by the `go` tool for finding
  dependencies.

* However, there are use cases where for better developer experience, it
  would make sense to have the dependencies reside in the same project
  folder as `etcd-druid`, so that tokens can be searched for, using
  project wide search in any text editor. Results can be fetched from
  the dependencies, without having to rely on the LSP.
  For example, this would make looking at all usages of a constant that
  is defined by a dependency easier.

* The developer can simply run `go mod vendor` without it affecting the
  modified build process which doesn't use `vendor`, as defined in gardener#748.
@renormalize renormalize requested a review from a team as a code owner April 19, 2024 14:32
@gardener-robot gardener-robot added area/dev-productivity Developer productivity related (how to improve development) kind/enhancement Enhancement, improvement, extension needs/review Needs review size/xs Size of pull request is tiny (see gardener-robot robot/bots/size.py) labels Apr 19, 2024
@gardener-robot-ci-1 gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Apr 19, 2024
@renormalize renormalize marked this pull request as draft April 19, 2024 14:33
@gardener-robot-ci-3 gardener-robot-ci-3 added needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Apr 19, 2024
@gardener-robot gardener-robot added the reviewed/do-not-merge Has no approval for merging as it may break things, be of poor quality or have (ext.) dependencies label Apr 19, 2024
@shreyas-s-rao shreyas-s-rao added this to the v0.23.0 milestone Apr 20, 2024
@gardener-robot-ci-3 gardener-robot-ci-3 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Apr 29, 2024
@renormalize renormalize marked this pull request as ready for review June 19, 2024 06:54
@renormalize
Copy link
Member Author

Can be merged once #777 merges.

Copy link
Contributor

@shreyas-s-rao shreyas-s-rao left a comment

Choose a reason for hiding this comment

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

/lgtm

@gardener-robot gardener-robot added reviewed/lgtm Has approval for merging and removed needs/review Needs review labels Jun 24, 2024
@gardener-robot-ci-3 gardener-robot-ci-3 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jun 24, 2024
@shreyas-s-rao shreyas-s-rao merged commit 277cbd1 into gardener:master Jun 24, 2024
8 checks passed
@gardener-robot gardener-robot added the status/closed Issue is closed (either delivered or triaged) label Jun 24, 2024
@gardener-robot-ci-2 gardener-robot-ci-2 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jun 24, 2024
@renormalize renormalize deleted the gitignore-vendor branch June 24, 2024 11:43
@renormalize renormalize self-assigned this Jul 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/dev-productivity Developer productivity related (how to improve development) kind/enhancement Enhancement, improvement, extension needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) reviewed/do-not-merge Has no approval for merging as it may break things, be of poor quality or have (ext.) dependencies reviewed/lgtm Has approval for merging size/xs Size of pull request is tiny (see gardener-robot robot/bots/size.py) status/closed Issue is closed (either delivered or triaged)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants