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 support for .nomad and .tfvars (HCL) #116

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

CalebAlbers
Copy link

HashiCorp Nomad users occasionally use the (discouraged) .nomad extension for HCL-based configurations. Likewise, .tfvars is frequently used in the Terraform realm. Both of these file formats follow the same form as HCL, just with differing extensions.

As an aside, both .hcl and tf files should ideally use the more idomatic # single-line format over single-line // or multi-line /* */ formats. I would love to change that in a future PR if you're open to it, but I recognize that it could come across as a disruptive change.

Thanks a ton for building this!

HashiCorp Nomad users occasionally use the (discouraged) `.nomad` extension for HCL-based configurations. Likewise, `.tfvars` is frequently used in the Terraform realm. Both of these file formats follow the same form as HCL, just with differing extensions.
@google-cla
Copy link

google-cla bot commented Jul 25, 2022

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@CalebAlbers
Copy link
Author

Side note - I'll have the CLA thing resolved soon - just put in the request to be covered under the HashiCorp CLA 🙂

@CalebAlbers
Copy link
Author

Okay - CLA is signed, so this should be all prepped for review now 🙂

@codecov-commenter
Copy link

codecov-commenter commented Jul 28, 2022

Codecov Report

Merging #116 (12435d4) into master (d40d757) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #116   +/-   ##
=======================================
  Coverage   48.60%   48.60%           
=======================================
  Files           2        2           
  Lines         251      251           
=======================================
  Hits          122      122           
  Misses        122      122           
  Partials        7        7           
Impacted Files Coverage Δ
main.go 41.62% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us.

@willnorris
Copy link
Collaborator

we've started to move away from all of the files in testdata (though obviously never finished). But could you go ahead and add this to our newer test case at https://github.com/google/addlicense/blob/master/main_test.go#L319 ?

@CalebAlbers
Copy link
Author

we've started to move away from all of the files in testdata (though obviously never finished). But could you go ahead and add this to our newer test case at https://github.com/google/addlicense/blob/master/main_test.go#L319 ?

Thanks @willnorris! I flipped to the test table strategy in 12435d4. Would you be open to me creating another PR to move the remaining file types into using that approach?

@willnorris
Copy link
Collaborator

Would you be open to me creating another PR to move the remaining file types into using that approach?

Yeah, that was always the intent. Most (all?) of the supported filetypes should already be covered by the new tests, but it's worth double checking. It's been a while since I looked at this, but I seem to recall there were still a few things the existing tests covered that needed to be migrated over. Maybe actually reading a file off disk, beyond just verifying that a given file extension maps to the write comment syntax? But yeah, if you wanna take a stab at this, feel free to open a new issue for finishing the test migration, and we can continue there.

@delete-merged-branch delete-merged-branch bot deleted the feat/hcl-aliases branch August 12, 2022 17:54
@CalebAlbers CalebAlbers restored the feat/hcl-aliases branch August 12, 2022 18:00
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.

None yet

3 participants