Skip to content

Conversation

@senthilhns
Copy link
Contributor

@senthilhns senthilhns commented Nov 26, 2024

Copy link
Member

@Ompragash Ompragash left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add ut's for the newly added functionalities + rename function names for clarity and the rest looks good
getGitCommittersCommand > buildGitLogCommand
getGitEmails > fetchCommitterEmails
cleanupAndSplitEmails > sanitizeEmailList

Copy link
Member

@Ompragash Ompragash left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed, please ensure to test the windows variant of the plugin image in Harness CI with the same steps as in Linux stage

also, add tests for the newly added functions for below cases:

  • repositories with no commits or committers without emails
  • slack user lookup failures or rate-limiting

Copy link
Member

@Ompragash Ompragash left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added some minor review - rest looks good

senthilhns and others added 5 commits December 6, 2024 18:05
Co-authored-by: OP (oppenheimer) <[email protected]>
Co-authored-by: OP (oppenheimer) <[email protected]>
Co-authored-by: OP (oppenheimer) <[email protected]>
Co-authored-by: OP (oppenheimer) <[email protected]>
@Ompragash Ompragash merged commit d45a460 into drone-plugins:master Dec 9, 2024
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.

3 participants