add support for cmake files #90
Conversation
Codecov Report
@@ Coverage Diff @@
## master #90 +/- ##
==========================================
+ Coverage 47.56% 48.60% +1.04%
==========================================
Files 2 2
Lines 246 251 +5
==========================================
+ Hits 117 122 +5
Misses 122 122
Partials 7 7
Continue to review full report at Codecov.
|
| func fileExtension(name string) string { | ||
| if v := filepath.Ext(name); v != "" { | ||
| return strings.ToLower(v) | ||
| return v |
There was a problem hiding this comment.
why not lowercase here, once, to avoid forcing all downstream users to do that? Are there one or more downstream users that need extension case preserved?
There was a problem hiding this comment.
well, there's only one downstream user right now, which is licenseHeader above. I have licenseHeader doing the lowercasing itself because it's going to need that anyway for #67. We could do it in both places, it'd just be a noop.
This matters less since there is only a single caller of this func, but I also kind of like the idea of this func not assuming what is going to be done with the file extension. In the one case we have, we are wanting to do a case-insensitive comparison, but that seems like it should be out of scope of a function whose job is just to return a file extension.
|
GitHub is having issues right now, so can't push new commits (https://www.githubstatus.com/) |
|
hmm, I was updating my suggested edit in #67 (comment) in light of these edits, and I'm now thinking it might be clearer to combine these changes. I'll repurpose this PR to just include cmake support as well, since I think that will make some things clearer. |
|
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
|
okay, take a look now. Sorry for the churn and complete repurposing of this PR 😄 Regarding CLA, co-author consent can be implied from #67. |
|
@googlebot I consent. |
|
@willnorris any updates on this PR? |
this refactors licenseHeader to make it easier to add additional logic to determine prefix, beyond just file extension.
Co-authored-by: Sergii Baitala <sbaitala@gmail.com>
c28acb8 to
9e09af5
Compare
|
rebased onto master. @mco-gh, you had a question about where we to the string lowercasing. See my comment from before and let me know what you think. |
|
looks good, thanks
…On Thu, 2 Sept 2021 at 21:08, Will Norris ***@***.***> wrote:
rebased onto master. @mco-gh <https://github.com/mco-gh>, you had a
question about where we to the string lowercasing. See my comment from
before and let me know what you think.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#90 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAFAXF4YMIAHZ42LVCWNY5TT77KSBANCNFSM5B3BUHRQ>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
--
Marc Cohen
Web: mco.dev
Email: ***@***.***
Working with me: mco.dev/working-with-marc
Feedback: How am I doing? Provide anonymous feedback!
<https://www.increment.me/feedback/049628fd-7339-406d-9203-17d3ec8976bd/submit>
Q: Why is this email three sentences or less?
A: three.sentenc.es
|
could you do a formal code review approval? That's required on the master branch. |
|
looks like PR is closed
…On Thu, 30 Sept 2021 at 20:39, Will Norris ***@***.***> wrote:
looks good, thanks
could you do a formal code review approval? That's required on the master
branch.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#90 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAFAXF2SPDYDUSB5JHUOO43UES4G3ANCNFSM5B3BUHRQ>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
--
Marc Cohen
Web: mco.dev
Email: ***@***.***
Working with me: mco.dev/working-with-marc
Feedback: How am I doing? Provide anonymous feedback!
<https://www.increment.me/feedback/049628fd-7339-406d-9203-17d3ec8976bd/submit>
Q: Why is this email three sentences or less?
A: three.sentenc.es
|
|
@willnorris any updates on this PR? |
Add support for cmake files, adapted from #67
Slight refactoring in
licenseHeader()andfileExtension()to make this and other future logic for determining license comment prefix a little cleaner.Closes #67