Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignore case of email address when counting different authors in commits #467

Merged

Conversation

Jagrutiti
Copy link
Contributor

@Jagrutiti Jagrutiti commented Jan 18, 2023

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or Jira
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Ensure you have provided tests - that demonstrates feature works or fixes the issue

Description

Updated code to fix case sensitivity issue of email ids.

Jira id

https://issues.jenkins.io/browse/JENKINS-70239

Copy link
Member

@uhafner uhafner left a comment

Choose a reason for hiding this comment

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

Best way to start an issue is:
Create a failing test case that exposes the problem

@@ -132,7 +132,7 @@ public String toString() {

private static int getDistinctCount(final Collection<? extends CommitDiffItem> commits,
final Function<CommitDiffItem, String> property) {
return (int) commits.stream().map(property).map(s -> String.toLowerCase(Locale.ENGLISH)).distinct().count();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Intellij threw the following error for this syntax:

Non-static method 'toLowerCase(java.util.Locale)' cannot be referenced from a static context

I changed it to the current one. And updated the test cases. Shall I write new one for this change?

Copy link
Member

Choose a reason for hiding this comment

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

You should write a failing test before you touch the code 🥲.

@Jagrutiti
Copy link
Contributor Author

Create a failing test case that exposes the problem

Do you want me to create this test case?

@codecov
Copy link

codecov bot commented Jan 18, 2023

Codecov Report

Merging #467 (4d981d9) into master (3d90265) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##             master     #467   +/-   ##
=========================================
  Coverage     71.42%   71.42%           
  Complexity      386      386           
=========================================
  Files            48       48           
  Lines          1169     1169           
  Branches        101      101           
=========================================
  Hits            835      835           
  Misses          293      293           
  Partials         41       41           
Impacted Files Coverage Δ
...kins/plugins/forensics/miner/CommitStatistics.java 100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@@ -132,7 +132,7 @@ public String toString() {

private static int getDistinctCount(final Collection<? extends CommitDiffItem> commits,
final Function<CommitDiffItem, String> property) {
return (int) commits.stream().map(property).map(s -> String.toLowerCase(Locale.ENGLISH)).distinct().count();
Copy link
Member

Choose a reason for hiding this comment

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

You should write a failing test before you touch the code 🥲.

@uhafner
Copy link
Member

uhafner commented Jan 18, 2023

Create a failing test case that exposes the problem

Do you want me to create this test case?

Yes

@uhafner uhafner added the bug Bugs or performance problems label Jan 19, 2023
@uhafner uhafner changed the title Case sensitivity on emails Ignore case of email address when counting different authors in commits Jan 19, 2023
@uhafner uhafner merged commit 69f021b into jenkinsci:master Jan 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bugs or performance problems
Projects
None yet
2 participants