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

chore: remove run-in-docker logic from Makefile #1514

Merged
merged 2 commits into from
Feb 9, 2024
Merged

Conversation

fnaranjo-vmw
Copy link
Contributor

The same benefits can be achieved by simply launching an interactive docker session inside a supported image and running commands from there. This approach has some additional benefits

  • Easier to maintain
  • Debloats Makefile
  • Less hidden magic
  • Same code for local and containerised execution

Checklist:

  • Have you added Release Notes in the docs repositories?
  • Have you ran make run-integration-tests and make run-terraform-tests?
  • Have you ran acceptance tests for the service under change?
  • Have you followed the Conventional Commits specification?

@cf-gitbot
Copy link

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/187011518

The labels on this github issue will be updated when the story is started.

README.md Outdated Show resolved Hide resolved
Makefile Outdated
####### broker environment variables
SECURITY_USER_NAME := $(or $(SECURITY_USER_NAME), aws-broker)
SECURITY_USER_PASSWORD := $(or $(SECURITY_USER_PASSWORD), aws-broker-pw)
GSB_PROVISION_DEFAULTS := $(or $(GSB_PROVISION_DEFAULTS), {"aws_vpc_id": "$(AWS_PAS_VPC_ID)"})

ifeq ($(GO_OK), 0) # use local go binary
GO=go
Copy link
Member

Choose a reason for hiding this comment

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

Potentially we can inline these variables as they now only have one value

fnaranjo-vmw and others added 2 commits February 9, 2024 11:12
The same benefits can be achieved by simply launching an
interactive docker session inside a supported image and running
commands from there. This approach has some additional benefits
- Easier to maintain
- Debloats Makefile
- Less hidden magic
- Same code for local and containerised execution
@blgm blgm force-pushed the chore-fnaranjo-cleanup branch from 62b6365 to b8eacf9 Compare February 9, 2024 11:28
@blgm blgm merged commit faec2e9 into main Feb 9, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

3 participants