-
Notifications
You must be signed in to change notification settings - Fork 7
Conversation
src/App/Fossa/Analyze.hs
Outdated
. M.map (T.pack . iso8601Show) | ||
. M.fromListWith max | ||
. mapMaybe readLine | ||
$ T.lines textContrib |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line got too long for me. ormolu suggested this style, so I went with it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me
src/App/Fossa/Analyze.hs
Outdated
uploadContributors baseUrl apiKey locator contributors | ||
|
||
|
||
fetchGitContributors :: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nit] can we move this to another module? (along with gitLogCmd
, of course)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have a name/existing module in mind?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd be okay with a new module named VCS.Git
or similar
8073b27
to
620c04e
Compare
@cnr would you mind rechecking? I fixed some merge conflicts and I'd like another set of eyes making sure I didn't miss something obvious. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still LGTM 👍
Basic outline:
{"<authorEmail>": "<latest contribution date: YYYY-MM-DD>", ...}
IO
:git log ...
outputgit log ...
are ignored, since git should be taking care of that. We can add protection here later if needed.