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

set default values for maintainer and vendor to hashicorp #14

Merged
merged 3 commits into from
Feb 2, 2023
Merged

set default values for maintainer and vendor to hashicorp #14

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f18e47a
set default values for maintainer and vendor to hashicorp
claire-labry Feb 2, 2023
d83bf6f
PR feedback
claire-labry Feb 2, 2023
3a0c55f
set vendor in input file
claire-labry Feb 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions action.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ inputs:
description: 'Maintainer name.'
default: ''
required: false
vendor:
Copy link

Choose a reason for hiding this comment

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

It would be good to add this to the inputs table in the readme too!

description: 'Vendor name'
default: ''
required: false
description:
description: 'Product description.'
default: ''
Expand Down
6 changes: 5 additions & 1 deletion fpm_template.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ type NfpmInput struct {
Arch string
Version string
Maintainer string
Vendor string
Description string
Homepage string
License string
Expand Down Expand Up @@ -76,6 +77,7 @@ func main() {
inputArch := os.Getenv("INPUT_ARCH")
inputVersion := os.Getenv("INPUT_VERSION")
inputMaintainer := os.Getenv("INPUT_MAINTAINER")
inputVendor := os.Getenv("INPUT_VENDOR")
inputDescription := os.Getenv("INPUT_DESCRIPTION")
inputHomepage := os.Getenv("INPUT_HOMEPAGE")
inputLicense := os.Getenv("INPUT_LICENSE")
Expand Down Expand Up @@ -110,6 +112,7 @@ func main() {
Arch: inputArch,
Version: inputVersion,
Maintainer: inputMaintainer,
Vendor: inputVendor,
Description: inputDescription,
Homepage: inputHomepage,
License: inputLicense,
Expand All @@ -136,7 +139,8 @@ arch: {{ .Arch }}
platform: linux
release: 1
version: {{ .Version }}
maintainer: {{ .Maintainer }}
maintainer: "HashiCorp"
Copy link

Choose a reason for hiding this comment

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

I think instead of hardcoding these here - we would want to add something to the above section for inputVendor and inputMaintainer to set these to HashiCorp unless a value has been passed in:

        inputVendor := os.Getenv("INPUT_VENDOR")
	if inputVendor == "" {
		inputVendor = "HashiCorp"
	}

Copy link

Choose a reason for hiding this comment

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

My personal opinion on this is we should set it as close to user input as possible so it is surfaced easier. So for example, rather than having the empty check in go code here, set the default in the input file https://github.com/hashicorp/actions-packaging-linux/pull/14/files#diff-1243c5424efaaa19bd8e813c5e6f6da46316e63761421b3e5f5c8ced9a36e6b6R33. This action should primarily be for us at HashiCorp to use internally but it seems more intuitive that users would check the input file because that's their interface with the action, rather than dig into the template file to check what the implementation details are and find out we are overwriting empty strings.

Copy link
Contributor

Choose a reason for hiding this comment

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

So the default is defined in GHA instead of the tool itself.

I like that. It also addresses the issue of opting out of the tag if one were to re-use the tool elsewhere.

vendor: "HashiCorp"
description: {{ .Description }}
homepage: {{ .Homepage }}
license: {{ .License }}
Expand Down