Skip to content

Conversation

@CalebAlbers
Copy link
Contributor

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
Contributor 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
Contributor 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
Contributor 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
@flwyd
Copy link
Contributor

flwyd commented Apr 26, 2025

Hi Caleb, thanks for contributing to addlicense. I don't have any experience with this filetype, so I'm curious about the best outcome here. In your initial comment you said "As an aside, both .hcl and tf files should ideally use the more idomatic # single-line format over single-line // or multi-line /* */ formats." It looks like the code change is using the # comment character, but only for the file extensions whose use isn't encouraged. This seems a little odd to me: if you use the good file extension you get the bad comment character, and vice versa.

It doesn't seem too disruptive to me to switch the comment character: any existing files with licenses will keep the comment character, so only new ones will get the change. Projects might end up with a mix of comment structures for their licenses, but the current status quo is that if a user has # comments, they'll end up with // comments at the top for the license, and thus a mix of comment styles within a single file.

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.

4 participants