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

Adds install-dependencies target in Makefile #351

Conversation

anvial
Copy link
Member

@anvial anvial commented Dec 4, 2023

Description

Add a new Makefile target, which adds the dependency on snaps listed in the PACKAGES variable.

Here is the list of the packages:

  • terraform
  • golangci-lint
  • go

Environment

  • Juju controller version: any

  • Terraform version: any

QA steps

make install-dependencies

Links

Jira-card: JUJU-5117

- adds the only dependancy on Terrform snap.
@anvial anvial requested a review from hmlanigan December 4, 2023 10:15
Copy link
Member

@hmlanigan hmlanigan 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, nice suggestion, a few comments.

Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
@hmlanigan
Copy link
Member

@anvial Could you update the Readme to reflect these changes.

@anvial anvial force-pushed the JUJU-5117-add-dependency-target-to-makefile-for-terraform-plugin branch from ae5f6ac to 7e3ca7f Compare December 8, 2023 07:45
Copy link
Member

@hmlanigan hmlanigan left a comment

Choose a reason for hiding this comment

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

One suggestion, but otherwise lgtm - thank you

@@ -34,6 +34,12 @@ _Note:_ These features may not have functional parity with the juju cli at this

1. Clone the repository
1. Enter the repository directory
1. Build the provider dependencies using the make `install-dependencies` target:
Copy link
Member

Choose a reason for hiding this comment

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

As we're not actually building the dependencies, suggest:

"Install the provider requirements and dependencies using the make install-dependencies target:"

@anvial anvial merged commit c588ca8 into juju:main Dec 11, 2023
9 checks passed
@anvial anvial deleted the JUJU-5117-add-dependency-target-to-makefile-for-terraform-plugin branch December 11, 2023 09:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants