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

🌱 Move tool dependencies into go.mod #460

Merged
merged 2 commits into from
May 17, 2021
Merged

🌱 Move tool dependencies into go.mod #460

merged 2 commits into from
May 17, 2021

Conversation

naveensrinivasan
Copy link
Member

  • Please check if the PR fulfills these requirements

  • What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
    Feature

  • What is the current behavior? (You can also link to an open issue here)
    Feature - manage tool dependencies via go.mod #459

  • What is the new behavior (if this is a feature change)?
    Moved the tool dependencies into go.mod

  • Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)
    None

  • Other information:
    None

@naveensrinivasan naveensrinivasan self-assigned this May 16, 2021
@github-actions
Copy link

Integration tests failure for 7ed0179145e0bb2a7eaa95badae404b9e162e928

@github-actions
Copy link

Integration tests failure for 2360d4db87853621d84d9dcbad2afd77439ec381

@github-actions
Copy link

Integration tests failure for 915d5835b61b304f5baeb1f21e9a1b8d1e3f2a2d

@github-actions
Copy link

Integration tests failure for d509df1c7221d7762b5a8e12729c265cbaf9dbab

@github-actions
Copy link

Integration tests failure for f118e6a79df9a9e6b68a507777f3016143eed923

@naveensrinivasan naveensrinivasan marked this pull request as draft May 16, 2021 21:57
azeemshaikh38
azeemshaikh38 reviewed May 17, 2021
View reviewed changes
Makefile Outdated

ci-e2e: ## Runs CI e2e tests. Requires GITHUB_AUTH_TOKEN env var to be set to GitHub personal access token
ci-e2e: build-scorecard check-env e2e-cron | $(GINKGO)
ci-e2e: build-scorecard check-env e2e-cron install| $(GINKGO)
Copy link
Contributor

Choose a reason for hiding this comment

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

Specifying install as dependency will always install all dependencies when we run make ci-e2e. Instead, we can have | $(GINKGO) and provide a rule for $(GINKGO) as:

$(GINKGO): install

That way, it'll only run the install command if $(GINKGO) file doesn't exist. Wdyt?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes agreed. Thanks!

@naveensrinivasan
🌱 Move tool dependencies into go.mod
349d3c3 
Moved the tool dependencies into go.mod
@naveensrinivasan naveensrinivasan marked this pull request as ready for review May 17, 2021 15:13
@github-actions
Copy link

Integration tests failure for c6f091e9d8d0d5a268615d8753c1f5e6503f972e

@naveensrinivasan naveensrinivasan force-pushed the feat/tools branch 2 times, most recently from 5c58803 to c0065dd Compare May 17, 2021 15:37
@github-actions
Copy link

Integration tests success for 5c58803748f62f777b54451ba94d6494310cd8f2

@github-actions
Copy link

Integration tests success for c0065dd56456224e8e285629809a7192afd5ca38

@github-actions
Copy link

Integration tests success for ccb4646d795f25fe42ec1f8e162d84645a8cb391

@naveensrinivasan naveensrinivasan linked an issue May 17, 2021 that may be closed by this pull request
BUG: Update build dependencies to use a hash/digest #429
Closed
@github-actions
Copy link

Integration tests success for 99fd5d3e40773b77badbde54616cfce3e5c63cea

@github-actions
Copy link

Integration tests success for 3a1a47c7972f61a9c697eb653867af1854d1fd60

@azeemshaikh38 azeemshaikh38 mentioned this pull request May 17, 2021
Closed
azeemshaikh38
azeemshaikh38 reviewed May 17, 2021
View reviewed changes
Makefile
check-linter: ## Install and run golang linter
check-linter: | $(GOLANGGCI_LINT)
check-linter: $(GOLANGGCI_LINT)
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be | $(GOLANGCI_LINT)? We only care about if this file exists or not.

@naveensrinivasan naveensrinivasan merged commit 9281d1d into main May 17, 2021
@naveensrinivasan naveensrinivasan deleted the feat/tools branch May 17, 2021 19:20
@ristomcgehee ristomcgehee mentioned this pull request Nov 14, 2021
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@azeemshaikh38 azeemshaikh38 azeemshaikh38 approved these changes

@inferno-chromium inferno-chromium Awaiting requested review from inferno-chromium

Assignees

@naveensrinivasan naveensrinivasan

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

BUG: Update build dependencies to use a hash/digest
2 participants
@naveensrinivasan @azeemshaikh38