add support for cmake files #90

willnorris · 2021-08-10T00:17:45Z

Add support for cmake files, adapted from #67

Slight refactoring in licenseHeader() and fileExtension() to make this and other future logic for determining license comment prefix a little cleaner.

Closes #67

codecov-commenter · 2021-08-10T00:18:15Z

Codecov Report

Merging #90 (9e09af5) into master (944049f) will increase coverage by 1.04%.
The diff coverage is 100.00%.

@@            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

Impacted Files	Coverage Δ
main.go	`41.62% <100.00%> (+1.43%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 944049f...9e09af5. Read the comment docs.

main.go

mco-gh · 2021-08-10T08:04:20Z

main.go

 func fileExtension(name string) string {
 	if v := filepath.Ext(name); v != "" {
-		return strings.ToLower(v)
+		return v


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?

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.

willnorris · 2021-08-10T16:22:12Z

GitHub is having issues right now, so can't push new commits (https://www.githubstatus.com/)

willnorris · 2021-08-10T16:44:50Z

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.

google-cla · 2021-08-10T16:51:16Z

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 @googlebot I consent. in this pull request.

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 cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

willnorris · 2021-08-10T16:55:53Z

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.

srgbtl · 2021-08-10T17:19:51Z

@googlebot I consent.

srgbtl · 2021-09-01T11:58:05Z

@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>

willnorris · 2021-09-02T20:07:50Z

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.

mco-gh · 2021-09-03T05:24:47Z

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

willnorris · 2021-09-30T19:39:14Z

looks good, thanks

could you do a formal code review approval? That's required on the master branch.

mco-gh · 2021-10-01T08:16:41Z

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 · 2021-10-02T16:59:16Z

#67 is closed, but this one (#90) is still open. This supersedes #67, so @srgbtl just went ahead and closed the other one preemptively.

srgbtl · 2021-11-15T15:29:10Z

@willnorris any updates on this PR?

mco-gh

Sorry about the delay

willnorris requested a review from mco-gh August 10, 2021 00:18

willnorris mentioned this pull request Aug 10, 2021

Support CMake files #67

Closed

mco-gh reviewed Aug 10, 2021

View reviewed changes

willnorris changed the title ~~refactor: prep for additional licenseHeader logic~~ add support for cmake files Aug 10, 2021

willnorris and others added 2 commits September 2, 2021 13:04

refactor: prep for additional licenseHeader logic

a44ae51

this refactors licenseHeader to make it easier to add additional logic to determine prefix, beyond just file extension.

feat: add support for cmake files

9e09af5

Co-authored-by: Sergii Baitala <sbaitala@gmail.com>

willnorris force-pushed the licenseheader branch from c28acb8 to 9e09af5 Compare September 2, 2021 20:06

willnorris requested a review from mco-gh November 15, 2021 15:31

mco-gh approved these changes Nov 15, 2021

View reviewed changes

willnorris merged commit 2b44b36 into google:master Nov 15, 2021

willnorris deleted the licenseheader branch November 15, 2021 16:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add support for cmake files #90

add support for cmake files #90

willnorris commented Aug 10, 2021 •

edited

Loading

codecov-commenter commented Aug 10, 2021 •

edited

Loading

mco-gh Aug 10, 2021

willnorris Aug 10, 2021

willnorris commented Aug 10, 2021

willnorris commented Aug 10, 2021

google-cla bot commented Aug 10, 2021

willnorris commented Aug 10, 2021

srgbtl commented Aug 10, 2021

srgbtl commented Sep 1, 2021

willnorris commented Sep 2, 2021

mco-gh commented Sep 3, 2021 via email

willnorris commented Sep 30, 2021

mco-gh commented Oct 1, 2021 via email

willnorris commented Oct 2, 2021

srgbtl commented Nov 15, 2021

mco-gh left a comment

add support for cmake files #90

add support for cmake files #90

Conversation

willnorris commented Aug 10, 2021 • edited Loading

codecov-commenter commented Aug 10, 2021 • edited Loading

Codecov Report

mco-gh Aug 10, 2021

Choose a reason for hiding this comment

willnorris Aug 10, 2021

Choose a reason for hiding this comment

willnorris commented Aug 10, 2021

willnorris commented Aug 10, 2021

google-cla bot commented Aug 10, 2021

willnorris commented Aug 10, 2021

srgbtl commented Aug 10, 2021

srgbtl commented Sep 1, 2021

willnorris commented Sep 2, 2021

mco-gh commented Sep 3, 2021 via email

willnorris commented Sep 30, 2021

mco-gh commented Oct 1, 2021 via email

willnorris commented Oct 2, 2021

srgbtl commented Nov 15, 2021

mco-gh left a comment

Choose a reason for hiding this comment

willnorris commented Aug 10, 2021 •

edited

Loading

codecov-commenter commented Aug 10, 2021 •

edited

Loading