From f3d47d72f5271b89b24f547d2c7fa01ab44ef626 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Fri, 26 Apr 2024 12:32:01 +0200 Subject: [PATCH 01/12] Fixes: 404 PRs and progressbar --- CHANGELOG.md | 12 ++++++++++-- gcl.java | 51 ++++++++++++++++++++++++++++++++++++++++++--------- 2 files changed, 52 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 81f4dff..0439f6e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,13 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). +## [2024-04-26] + +### Fixed + +- Fixed handling of 404 pull requests. [#11](https://github.com/koppor/github-contributors-list/issues/11) +- Fixed progressbar going backwards when using defaults. + ## [2024-04-25] ### Added @@ -14,11 +21,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - Can now run without any parameters: The current directory is used as the repository path. - Use `--repository owner/repo` instead of `--owner` and `--repo`. -- The defaults for `--startrevision` and `--endrevision` are now the complete branch history (starting from `HEAD`). +- The defaults for `--startrevision` and `--endrevision` are now the complete branch history (starting from `HEAD`). [#3](https://github.com/koppor/github-contributors-list/issues/3) ### Fixed -- Commits without any pull request number are also analyzed. +- Commits without any pull request number are also analyzed. [#4](https://github.com/koppor/github-contributors-list/issues/4) - Fixed typo in parameter `--login-mapping`. - Handling of ignored users. @@ -30,5 +37,6 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). Initial release. +[2024-04-26]: https://github.com/koppor/github-contributors-list/compare/2024-04-25...2024-04-26 [2024-04-25]: https://github.com/koppor/github-contributors-list/compare/2024-04-09...2024-04-25 [2024-04-09]: https://github.com/koppor/github-contributors-list/releases/tag/2024-04-09 diff --git a/gcl.java b/gcl.java index 957cfce..ed36e1e 100755 --- a/gcl.java +++ b/gcl.java @@ -21,6 +21,7 @@ import java.nio.file.Path; import java.time.Duration; import java.time.Instant; +import java.time.ZoneId; import java.time.format.DateTimeFormatter; import java.util.Comparator; import java.util.HashSet; @@ -40,8 +41,10 @@ import one.util.streamex.StreamEx; import org.eclipse.collections.api.multimap.MutableMultimap; import org.eclipse.collections.impl.factory.Multimaps; +import org.eclipse.jgit.api.errors.GitAPIException; import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.revwalk.RevSort; +import org.kohsuke.github.GHFileNotFoundException; import org.tinylog.Logger; import org.eclipse.jgit.api.Git; @@ -236,8 +239,20 @@ public Integer call() throws Exception { Instant startDate = startCommit.getAuthorIdent().getWhen().toInstant(); Instant endDate = endCommit.getAuthorIdent().getWhen().toInstant(); + if (startDate.isAfter(endDate)) { + Logger.warn("Start date is after end date. Swapping."); + // Swap start and end date + Instant tmp = endDate; + endDate = startDate; + startDate = tmp; + } + long days = Duration.between(startDate, endDate).toDays(); - Logger.info("Analyzing {} days backwards: From {} to {}", days, DateTimeFormatter.ISO_INSTANT.format(endDate), DateTimeFormatter.ISO_INSTANT.format(startDate)); + DateTimeFormatter isoDateTimeFormatter = DateTimeFormatter.ISO_LOCAL_DATE.withZone(ZoneId.systemDefault()); + Logger.info("Analyzing {} days backwards: From {} to {}", + days, + isoDateTimeFormatter.format(startDate), + isoDateTimeFormatter.format(endDate)); try (ProgressBar pb = new ProgressBar("Analyzing", (int) days)) { pb.maxHint((int) days); @@ -291,9 +306,9 @@ private void outputFallbacks() { * Analyzes a commit found in the repository. Will then diverge to the pull request analysis if a PR number is found in the commit message. Otherwise, the commit is analyzed as a regular commit. */ private void analyzeCommit(long days, Instant startDate, RevCommit commit, ProgressBar progressBar, GHRepository ghRepository, MVMap loginToContributor, MVMap emailToContributor, GitHub gitHub) throws IOException { - long daysSinceLast = days - Duration.between(startDate, commit.getAuthorIdent().getWhen().toInstant()).toDays(); - Logger.trace("{} daysSinceLast", daysSinceLast); - progressBar.stepTo((int) daysSinceLast); + long daysSinceFirstCommit = Duration.between(startDate, commit.getAuthorIdent().getWhen().toInstant()).toDays(); + Logger.trace("{} daysSinceFirstCommit", daysSinceFirstCommit); + progressBar.stepTo((int) daysSinceFirstCommit); String shortMessage = commit.getShortMessage(); Logger.trace("Checking commit \"{}\" ({})", shortMessage, commit.getId().name()); @@ -311,19 +326,35 @@ private void analyzeCommit(long days, Instant startDate, RevCommit commit, Progr } if (number == null) { Logger.trace("No PR number found in commit message"); - CoAuthor authorOfCommit = new CoAuthor(commit.getAuthorIdent().getName(), commit.getAuthorIdent().getEmailAddress()); - analyzeRegularCommit(authorOfCommit, emailToContributor, gitHub, commit.getFullMessage()); + addContributorFromRevCommit(commit, emailToContributor, gitHub); } else { - analyzePullRequest(progressBar, ghRepository, loginToContributor, emailToContributor, gitHub, number); + if (!analyzePullRequest(progressBar, ghRepository, loginToContributor, emailToContributor, gitHub, number)) { + Logger.trace("PR was 404. Interpreting commit itself"); + addContributorFromRevCommit(commit, emailToContributor, gitHub); + } } } - private void analyzePullRequest(ProgressBar progressBar, GHRepository ghRepository, MVMap loginToContributor, MVMap emailToContributor, GitHub gitHub, String number) throws IOException { + private void addContributorFromRevCommit(RevCommit commit, MVMap emailToContributor, GitHub gitHub) { + CoAuthor authorOfCommit = new CoAuthor(commit.getAuthorIdent().getName(), commit.getAuthorIdent().getEmailAddress()); + analyzeRegularCommit(authorOfCommit, emailToContributor, gitHub, commit.getFullMessage()); + } + + /** + * @return false if the PR did not exist + */ + private boolean analyzePullRequest(ProgressBar progressBar, GHRepository ghRepository, MVMap loginToContributor, MVMap emailToContributor, GitHub gitHub, String number) throws IOException { Logger.trace("Investigating PR #{}", number); this.currentPR = number; progressBar.setExtraMessage("PR " + number); int prNumber = Integer.parseInt(number); - GHPullRequest pullRequest = ghRepository.getPullRequest(prNumber); + GHPullRequest pullRequest; + try { + pullRequest = ghRepository.getPullRequest(prNumber); + } catch (GHFileNotFoundException e) { + Logger.warn("Pull request {} not found", prNumber); + return false; + } GHUser user = pullRequest.getUser(); storeContributorData(loginToContributor, emailToContributor, user); @@ -342,6 +373,8 @@ private void analyzePullRequest(ProgressBar progressBar, GHRepository ghReposito CoAuthor authorOfCommit = new CoAuthor(theCommit.getAuthor().getName(), theCommit.getAuthor().getEmail()); analyzeRegularCommit(authorOfCommit, emailToContributor, gitHub, theCommit.getMessage()); } + + return true; } private void analyzeRegularCommit(CoAuthor authorOfCommit, MVMap emailToContributor, GitHub gitHub, String commitMessage) { From bb7b912c1ab90f9947055dac461c1a82d6f0fd4b Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Fri, 26 Apr 2024 14:21:15 +0200 Subject: [PATCH 02/12] Extract method --- gcl.java | 151 +++++++++++++++++++++++++++++++------------------------ 1 file changed, 84 insertions(+), 67 deletions(-) diff --git a/gcl.java b/gcl.java index ed36e1e..74004a1 100755 --- a/gcl.java +++ b/gcl.java @@ -184,39 +184,84 @@ public Integer call() throws Exception { fileName("gcl.mv"). open(); Git git = Git.open(repositoryPath.toFile()); - Repository repository = git.getRepository(); RevWalk revWalk = new RevWalk(repository)) { + Repository repository = git.getRepository()) { - String remoteOriginUrl = "n/a"; - if (!hasRepository) { - // Source: https://stackoverflow.com/a/38062680/873282 - remoteOriginUrl = git.getRepository().getConfig().getString("remote", "origin", "url"); - if (remoteOriginUrl.startsWith("git@")) { - ownerRepository = remoteOriginUrl.substring(remoteOriginUrl.indexOf(':') + 1, remoteOriginUrl.lastIndexOf('.')); - } else { - ownerRepository = remoteOriginUrl.substring(remoteOriginUrl.indexOf("github.com/") + "github.com/".length()); - if (ownerRepository.endsWith(".git")) { - ownerRepository = ownerRepository.substring(0, ownerRepository.length() - ".git".length()); + RepositoryData repositoryData = getRepositoryData(git, store, repository); + if (repositoryData == null) { + return 1; + } + + DateTimeFormatter isoDateTimeFormatter = DateTimeFormatter.ISO_LOCAL_DATE.withZone(ZoneId.systemDefault()); + Logger.info("Analyzing {} days: From {} to {}", + repositoryData.days(), + isoDateTimeFormatter.format(repositoryData.startDate()), + isoDateTimeFormatter.format(repositoryData.endDate())); + + MVMap loginToContributor = store.openMap("loginToContributor"); + MVMap emailToContributor = store.openMap("emailToContributor"); + + try (ProgressBar pb = new ProgressBar("Analyzing", (int) repositoryData.days()); + RevWalk revWalk = new RevWalk(repository)) { + revWalk.sort(RevSort.REVERSE); + revWalk.markStart(repositoryData.startCommit()); + Iterator commitIterator = revWalk.iterator(); + + while (commitIterator.hasNext()) { + RevCommit commit = commitIterator.next(); + analyzeCommit(pb, repositoryData.gitHub(), repositoryData.gitHubRepository(), loginToContributor, emailToContributor, repositoryData.startDate(), commit); + if (commit.equals(repositoryData.endCommit())) { + break; } } } + } - Logger.info("Connecting to {}...", ownerRepository); - GitHub gitHub = GitHub.connect(); - GHRepository gitHubRepository; - try { - gitHubRepository = gitHub.getRepository(ownerRepository); - } catch (IllegalArgumentException e) { - Logger.error("Error in repository reference {}", ownerRepository); - if (!hasRepository) { - Logger.error("It was automatically derived from {}", remoteOriginUrl); + Logger.debug("Found contributors {}", contributors); + Logger.debug("Used fallbacks {}", fallbacks); + Logger.debug("Fallback source {}", fallbackSources); + + if (!fallbacks.isEmpty()) { + outputFallbacks(); + } + + System.out.println(); + + printMarkdownSnippet(); + + return 0; + } + + private RepositoryData getRepositoryData(Git git, MVStore store, Repository repository) throws IOException, GitAPIException { + String remoteOriginUrl = "n/a"; + if (!hasRepository) { + // Source: https://stackoverflow.com/a/38062680/873282 + remoteOriginUrl = git.getRepository().getConfig().getString("remote", "origin", "url"); + if (remoteOriginUrl.startsWith("git@")) { + ownerRepository = remoteOriginUrl.substring(remoteOriginUrl.indexOf(':') + 1, remoteOriginUrl.lastIndexOf('.')); + } else { + ownerRepository = remoteOriginUrl.substring(remoteOriginUrl.indexOf("github.com/") + "github.com/".length()); + if (ownerRepository.endsWith(".git")) { + ownerRepository = ownerRepository.substring(0, ownerRepository.length() - ".git".length()); } - return 1; } + } - MVMap emailToContributor = store.openMap("emailToContributor"); - MVMap loginToContributor = store.openMap("loginToContributor"); + Logger.info("Connecting to {}...", ownerRepository); + GitHub gitHub = GitHub.connect(); + GHRepository gitHubRepository; + try { + gitHubRepository = gitHub.getRepository(ownerRepository); + } catch (IllegalArgumentException e) { + Logger.error("Error in repository reference {}", ownerRepository); + if (!hasRepository) { + Logger.error("It was automatically derived from {}", remoteOriginUrl); + } + return null; + } - RevCommit startCommit; + RevCommit startCommit; + RevCommit endCommit; + try (RevWalk revWalk = new RevWalk(repository)) { if (hasStartRevision) { startCommit = revWalk.parseCommit(repository.resolve(startCommitRevStr)); } else { @@ -227,7 +272,6 @@ public Integer call() throws Exception { Logger.trace("No explicit start commit, using {}", startCommit.getId().name()); } - RevCommit endCommit; if (hasEndRevision) { endCommit = revWalk.parseCommit(repository.resolve(endCommitRevStr)); } else { @@ -236,53 +280,25 @@ public Integer call() throws Exception { Logger.trace("No explicit end commit, using {}", endCommit.getId().name()); } - Instant startDate = startCommit.getAuthorIdent().getWhen().toInstant(); - Instant endDate = endCommit.getAuthorIdent().getWhen().toInstant(); - - if (startDate.isAfter(endDate)) { - Logger.warn("Start date is after end date. Swapping."); - // Swap start and end date - Instant tmp = endDate; - endDate = startDate; - startDate = tmp; - } - - long days = Duration.between(startDate, endDate).toDays(); - DateTimeFormatter isoDateTimeFormatter = DateTimeFormatter.ISO_LOCAL_DATE.withZone(ZoneId.systemDefault()); - Logger.info("Analyzing {} days backwards: From {} to {}", - days, - isoDateTimeFormatter.format(startDate), - isoDateTimeFormatter.format(endDate)); - - try (ProgressBar pb = new ProgressBar("Analyzing", (int) days)) { - pb.maxHint((int) days); - - revWalk.markStart(endCommit); - Iterator commitIterator = revWalk.iterator(); - - while (commitIterator.hasNext()) { - RevCommit commit = commitIterator.next(); - if (commit.equals(startCommit)) { - break; - } - analyzeCommit(days, startDate, commit, pb, gitHubRepository, loginToContributor, emailToContributor, gitHub); - } - } } - Logger.debug("Found contributors {}", contributors); - Logger.debug("Used fallbacks {}", fallbacks); - Logger.debug("Fallback source {}", fallbackSources); + Instant startDate = startCommit.getAuthorIdent().getWhen().toInstant(); + Instant endDate = endCommit.getAuthorIdent().getWhen().toInstant(); - if (!fallbacks.isEmpty()) { - outputFallbacks(); + if (startDate.isAfter(endDate)) { + Logger.warn("Start date is after end date. Swapping."); + // Swap start and end date + Instant tmp = endDate; + endDate = startDate; + startDate = tmp; } - System.out.println(); + long days = Duration.between(startDate, endDate).toDays(); - printMarkdownSnippet(); + return new RepositoryData(gitHub, gitHubRepository, startCommit, startDate, endCommit, endDate, days); + } - return 0; + private record RepositoryData(GitHub gitHub, GHRepository gitHubRepository, RevCommit startCommit, Instant startDate, RevCommit endCommit, Instant endDate, long days) { } private void outputFallbacks() { @@ -305,10 +321,11 @@ private void outputFallbacks() { /** * Analyzes a commit found in the repository. Will then diverge to the pull request analysis if a PR number is found in the commit message. Otherwise, the commit is analyzed as a regular commit. */ - private void analyzeCommit(long days, Instant startDate, RevCommit commit, ProgressBar progressBar, GHRepository ghRepository, MVMap loginToContributor, MVMap emailToContributor, GitHub gitHub) throws IOException { + private void analyzeCommit(ProgressBar progressBar, GitHub gitHub, GHRepository ghRepository, MVMap loginToContributor, MVMap emailToContributor, Instant startDate, RevCommit commit) throws IOException { long daysSinceFirstCommit = Duration.between(startDate, commit.getAuthorIdent().getWhen().toInstant()).toDays(); Logger.trace("{} daysSinceFirstCommit", daysSinceFirstCommit); - progressBar.stepTo((int) daysSinceFirstCommit); + + progressBar.stepTo(Math.max((int) daysSinceFirstCommit, progressBar.getCurrent())); String shortMessage = commit.getShortMessage(); Logger.trace("Checking commit \"{}\" ({})", shortMessage, commit.getId().name()); From d43e55b34605c96fcbbd751e63c6113965e8a125 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Fri, 26 Apr 2024 14:21:23 +0200 Subject: [PATCH 03/12] Try to fix parsing --- gcl.java | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/gcl.java b/gcl.java index 74004a1..3b0af31 100755 --- a/gcl.java +++ b/gcl.java @@ -192,26 +192,25 @@ public Integer call() throws Exception { } DateTimeFormatter isoDateTimeFormatter = DateTimeFormatter.ISO_LOCAL_DATE.withZone(ZoneId.systemDefault()); - Logger.info("Analyzing {} days: From {} to {}", + Logger.info("Analyzing {} days backwards: From {} to {}", repositoryData.days(), - isoDateTimeFormatter.format(repositoryData.startDate()), - isoDateTimeFormatter.format(repositoryData.endDate())); + isoDateTimeFormatter.format(repositoryData.endDate()), + isoDateTimeFormatter.format(repositoryData.startDate())); MVMap loginToContributor = store.openMap("loginToContributor"); MVMap emailToContributor = store.openMap("emailToContributor"); try (ProgressBar pb = new ProgressBar("Analyzing", (int) repositoryData.days()); RevWalk revWalk = new RevWalk(repository)) { - revWalk.sort(RevSort.REVERSE); - revWalk.markStart(repositoryData.startCommit()); + revWalk.markStart(repositoryData.endCommit()); Iterator commitIterator = revWalk.iterator(); while (commitIterator.hasNext()) { RevCommit commit = commitIterator.next(); - analyzeCommit(pb, repositoryData.gitHub(), repositoryData.gitHubRepository(), loginToContributor, emailToContributor, repositoryData.startDate(), commit); - if (commit.equals(repositoryData.endCommit())) { + if (commit.equals(repositoryData.startCommit())) { break; } + analyzeCommit(pb, repositoryData.gitHub(), repositoryData.gitHubRepository(), loginToContributor, emailToContributor, repositoryData.startDate(), commit); } } } From 7fda917e0c2f2659a5f7134a719148fe9474a73c Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Fri, 26 Apr 2024 14:50:15 +0200 Subject: [PATCH 04/12] Remove unused method --- gcl.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/gcl.java b/gcl.java index 3b0af31..1277358 100755 --- a/gcl.java +++ b/gcl.java @@ -168,9 +168,6 @@ public static void main(String... args) throws Exception { System.exit(exitCode); } - private static void setDefaultValues(String[] args, CommandLine commandLine) { - } - @Override public Integer call() throws Exception { Logger.info("Opening local git repository {}...", repositoryPath); From 6c35319ca50726d2b95a937f711357b78d6a2c52 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Fri, 26 Apr 2024 15:30:52 +0200 Subject: [PATCH 05/12] Reformat, split up, fixes --- gcl.java | 153 +++++++++++++++++++++++++++---------------------------- 1 file changed, 75 insertions(+), 78 deletions(-) diff --git a/gcl.java b/gcl.java index 1277358..f87471d 100755 --- a/gcl.java +++ b/gcl.java @@ -41,18 +41,16 @@ import one.util.streamex.StreamEx; import org.eclipse.collections.api.multimap.MutableMultimap; import org.eclipse.collections.impl.factory.Multimaps; +import org.eclipse.jgit.api.Git; import org.eclipse.jgit.api.errors.GitAPIException; import org.eclipse.jgit.lib.Constants; -import org.eclipse.jgit.revwalk.RevSort; -import org.kohsuke.github.GHFileNotFoundException; -import org.tinylog.Logger; - -import org.eclipse.jgit.api.Git; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revwalk.RevCommit; +import org.eclipse.jgit.revwalk.RevSort; import org.eclipse.jgit.revwalk.RevWalk; import org.h2.mvstore.MVMap; import org.h2.mvstore.MVStore; +import org.kohsuke.github.GHFileNotFoundException; import org.kohsuke.github.GHPullRequest; import org.kohsuke.github.GHPullRequestCommitDetail; import org.kohsuke.github.GHRepository; @@ -60,7 +58,7 @@ import org.kohsuke.github.GitHub; import org.kohsuke.github.PagedIterator; import org.kohsuke.github.PagedSearchIterable; - +import org.tinylog.Logger; import picocli.CommandLine; import picocli.CommandLine.Command; import picocli.CommandLine.Option; @@ -92,7 +90,7 @@ public class gcl implements Callable { private String ownerRepository; @Option(names = "--cols", description = "Number of columns") - private Integer cols = 6; + private Integer cols = 6; @Option(names = "--filter") private List ignoredUsers = List.of( @@ -112,7 +110,7 @@ public class gcl implements Callable { "gradle-update-robot@regolo.cc", "team@moderne.io"); - @Option(names = { "-l", "--github-lookup" }, description = "Should calls be made to GitHub's API for user information", negatable = true) + @Option(names = {"-l", "--github-lookup"}, description = "Should calls be made to GitHub's API for user information", negatable = true) boolean ghLookup = true; @Option(names = {"-m", "--login-mapping"}, description = {"Mapping of GitHub logins to names. Format: name=login"}) @@ -180,34 +178,50 @@ public Integer call() throws Exception { try (MVStore store = new MVStore.Builder(). fileName("gcl.mv"). open(); - Git git = Git.open(repositoryPath.toFile()); - Repository repository = git.getRepository()) { + Git git = Git.open(repositoryPath.toFile()); + Repository repository = git.getRepository()) { - RepositoryData repositoryData = getRepositoryData(git, store, repository); - if (repositoryData == null) { - return 1; + String remoteOriginUrl = "n/a"; + if (!hasRepository) { + // Source: https://stackoverflow.com/a/38062680/873282 + remoteOriginUrl = git.getRepository().getConfig().getString("remote", "origin", "url"); + if (remoteOriginUrl.startsWith("git@")) { + ownerRepository = remoteOriginUrl.substring(remoteOriginUrl.indexOf(':') + 1, remoteOriginUrl.lastIndexOf('.')); + } else { + ownerRepository = remoteOriginUrl.substring(remoteOriginUrl.indexOf("github.com/") + "github.com/".length()); + if (ownerRepository.endsWith(".git")) { + ownerRepository = ownerRepository.substring(0, ownerRepository.length() - ".git".length()); + } + } } - DateTimeFormatter isoDateTimeFormatter = DateTimeFormatter.ISO_LOCAL_DATE.withZone(ZoneId.systemDefault()); - Logger.info("Analyzing {} days backwards: From {} to {}", - repositoryData.days(), - isoDateTimeFormatter.format(repositoryData.endDate()), - isoDateTimeFormatter.format(repositoryData.startDate())); + Logger.info("Connecting to {}...", ownerRepository); + GitHub gitHub = GitHub.connect(); + GHRepository gitHubRepository; + try { + gitHubRepository = gitHub.getRepository(ownerRepository); + } catch (IllegalArgumentException e) { + Logger.error("Error in repository reference {}", ownerRepository); + if (!hasRepository) { + Logger.error("It was automatically derived from {}", remoteOriginUrl); + } + return 1; + } + CommitRangeInformation commitRangeInformation = getCommitRangeInformation(repository, git); MVMap loginToContributor = store.openMap("loginToContributor"); MVMap emailToContributor = store.openMap("emailToContributor"); - - try (ProgressBar pb = new ProgressBar("Analyzing", (int) repositoryData.days()); - RevWalk revWalk = new RevWalk(repository)) { - revWalk.markStart(repositoryData.endCommit()); + // We need a completely new RevWalk object to have RevSort.REVERSE working + // MWE shown at https://stackoverflow.com/a/78390567/873282. + try (RevWalk revWalk = new RevWalk(repository); + ProgressBar progressBar = new ProgressBar("Analyzing", (int) commitRangeInformation.days())) { + revWalk.markUninteresting(revWalk.parseCommit(repository.resolve(commitRangeInformation.startCommitId()))); + revWalk.markStart(revWalk.parseCommit(repository.resolve(commitRangeInformation.endCommitId()))); + revWalk.sort(RevSort.REVERSE); Iterator commitIterator = revWalk.iterator(); - while (commitIterator.hasNext()) { RevCommit commit = commitIterator.next(); - if (commit.equals(repositoryData.startCommit())) { - break; - } - analyzeCommit(pb, repositoryData.gitHub(), repositoryData.gitHubRepository(), loginToContributor, emailToContributor, repositoryData.startDate(), commit); + analyzeCommit(progressBar, gitHub, gitHubRepository, loginToContributor, emailToContributor, commitRangeInformation.startDate(), commit); } } } @@ -227,36 +241,11 @@ public Integer call() throws Exception { return 0; } - private RepositoryData getRepositoryData(Git git, MVStore store, Repository repository) throws IOException, GitAPIException { - String remoteOriginUrl = "n/a"; - if (!hasRepository) { - // Source: https://stackoverflow.com/a/38062680/873282 - remoteOriginUrl = git.getRepository().getConfig().getString("remote", "origin", "url"); - if (remoteOriginUrl.startsWith("git@")) { - ownerRepository = remoteOriginUrl.substring(remoteOriginUrl.indexOf(':') + 1, remoteOriginUrl.lastIndexOf('.')); - } else { - ownerRepository = remoteOriginUrl.substring(remoteOriginUrl.indexOf("github.com/") + "github.com/".length()); - if (ownerRepository.endsWith(".git")) { - ownerRepository = ownerRepository.substring(0, ownerRepository.length() - ".git".length()); - } - } - } - - Logger.info("Connecting to {}...", ownerRepository); - GitHub gitHub = GitHub.connect(); - GHRepository gitHubRepository; - try { - gitHubRepository = gitHub.getRepository(ownerRepository); - } catch (IllegalArgumentException e) { - Logger.error("Error in repository reference {}", ownerRepository); - if (!hasRepository) { - Logger.error("It was automatically derived from {}", remoteOriginUrl); - } - return null; - } - + private CommitRangeInformation getCommitRangeInformation(Repository repository, Git git) throws IOException, GitAPIException { RevCommit startCommit; RevCommit endCommit; + Instant startDate; + long days; try (RevWalk revWalk = new RevWalk(repository)) { if (hasStartRevision) { startCommit = revWalk.parseCommit(repository.resolve(startCommitRevStr)); @@ -276,25 +265,31 @@ private RepositoryData getRepositoryData(Git git, MVStore store, Repository repo Logger.trace("No explicit end commit, using {}", endCommit.getId().name()); } - } + startDate = startCommit.getAuthorIdent().getWhen().toInstant(); + Instant endDate = endCommit.getAuthorIdent().getWhen().toInstant(); - Instant startDate = startCommit.getAuthorIdent().getWhen().toInstant(); - Instant endDate = endCommit.getAuthorIdent().getWhen().toInstant(); + if (startDate.isAfter(endDate)) { + Logger.warn("Start date is after end date. Swapping."); + // Swap start and end date + Instant tmp = endDate; + endDate = startDate; + startDate = tmp; + } - if (startDate.isAfter(endDate)) { - Logger.warn("Start date is after end date. Swapping."); - // Swap start and end date - Instant tmp = endDate; - endDate = startDate; - startDate = tmp; - } + days = Duration.between(startDate, endDate).toDays(); + + DateTimeFormatter isoDateTimeFormatter = DateTimeFormatter.ISO_LOCAL_DATE.withZone(ZoneId.systemDefault()); + Logger.info("Analyzing {} days: From {} to {}", + days, + isoDateTimeFormatter.format(endDate), + isoDateTimeFormatter.format(startDate)); - long days = Duration.between(startDate, endDate).toDays(); + } - return new RepositoryData(gitHub, gitHubRepository, startCommit, startDate, endCommit, endDate, days); + return new CommitRangeInformation(startCommit.name(), startDate, days, endCommit.name()); } - private record RepositoryData(GitHub gitHub, GHRepository gitHubRepository, RevCommit startCommit, Instant startDate, RevCommit endCommit, Instant endDate, long days) { + private record CommitRangeInformation(String startCommitId, Instant startDate, long days, String endCommitId) { } private void outputFallbacks() { @@ -316,6 +311,8 @@ private void outputFallbacks() { /** * Analyzes a commit found in the repository. Will then diverge to the pull request analysis if a PR number is found in the commit message. Otherwise, the commit is analyzed as a regular commit. + * + * @param progressBar the progress bar - required display the progress of the analysis */ private void analyzeCommit(ProgressBar progressBar, GitHub gitHub, GHRepository ghRepository, MVMap loginToContributor, MVMap emailToContributor, Instant startDate, RevCommit commit) throws IOException { long daysSinceFirstCommit = Duration.between(startDate, commit.getAuthorIdent().getWhen().toInstant()).toDays(); @@ -341,7 +338,7 @@ private void analyzeCommit(ProgressBar progressBar, GitHub gitHub, GHRepository Logger.trace("No PR number found in commit message"); addContributorFromRevCommit(commit, emailToContributor, gitHub); } else { - if (!analyzePullRequest(progressBar, ghRepository, loginToContributor, emailToContributor, gitHub, number)) { + if (!analyzePullRequest(progressBar, ghRepository, loginToContributor, emailToContributor, gitHub, number, commit.getName())) { Logger.trace("PR was 404. Interpreting commit itself"); addContributorFromRevCommit(commit, emailToContributor, gitHub); } @@ -356,7 +353,7 @@ private void addContributorFromRevCommit(RevCommit commit, MVMap loginToContributor, MVMap emailToContributor, GitHub gitHub, String number) throws IOException { + private boolean analyzePullRequest(ProgressBar progressBar, GHRepository ghRepository, MVMap loginToContributor, MVMap emailToContributor, GitHub gitHub, String number, String prCommitNumber) throws IOException { Logger.trace("Investigating PR #{}", number); this.currentPR = number; progressBar.setExtraMessage("PR " + number); @@ -365,7 +362,7 @@ private boolean analyzePullRequest(ProgressBar progressBar, GHRepository ghRepos try { pullRequest = ghRepository.getPullRequest(prNumber); } catch (GHFileNotFoundException e) { - Logger.warn("Pull request {} not found", prNumber); + Logger.warn("Pull request {} not found. Referenced from commit {}.", prNumber, prCommitNumber); return false; } GHUser user = pullRequest.getUser(); @@ -397,12 +394,12 @@ private void analyzeRegularCommit(CoAuthor authorOfCommit, MVMap line.startsWith("Co-authored-by:")) - .map(CoAuthor::new) - .map(coAuthor -> lookupContributorData(emailToContributor, gitHub, coAuthor)) - .filter(Optional::isPresent) - .map(Optional::get) - .forEach(contributors::add); + .filter(line -> line.startsWith("Co-authored-by:")) + .map(CoAuthor::new) + .map(coAuthor -> lookupContributorData(emailToContributor, gitHub, coAuthor)) + .filter(Optional::isPresent) + .map(Optional::get) + .forEach(contributors::add); } private void printMarkdownSnippet() { @@ -442,7 +439,7 @@ private static String getFormattedFirstLine(Contributor contributor) { return contributor.name(); } return """ - [%s](%3$s)""".formatted(contributor.name(), contributor.avatarUrl(), contributor.url(), avatarImgWidth); + [%s](%3$s)""".formatted(contributor.name(), contributor.avatarUrl(), contributor.url(), avatarImgWidth); } private static String getFormattedSecondLine(Contributor contributor) { From 188b1baf9ae806d9c67d6321c0065b191540eb6c Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Fri, 26 Apr 2024 15:31:55 +0200 Subject: [PATCH 06/12] Fix date output --- gcl.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gcl.java b/gcl.java index f87471d..28c827f 100755 --- a/gcl.java +++ b/gcl.java @@ -281,8 +281,8 @@ private CommitRangeInformation getCommitRangeInformation(Repository repository, DateTimeFormatter isoDateTimeFormatter = DateTimeFormatter.ISO_LOCAL_DATE.withZone(ZoneId.systemDefault()); Logger.info("Analyzing {} days: From {} to {}", days, - isoDateTimeFormatter.format(endDate), - isoDateTimeFormatter.format(startDate)); + isoDateTimeFormatter.format(startDate), + isoDateTimeFormatter.format(endDate)); } From 81c58fe4c1c3534ac56dfaca2382ca00258172ea Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Fri, 26 Apr 2024 16:02:28 +0200 Subject: [PATCH 07/12] Add more tracing information --- CHANGELOG.md | 4 ++++ gcl.java | 53 +++++++++++++++++++++++++++------------------------- 2 files changed, 32 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0439f6e..11c1ee0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - Fixed handling of 404 pull requests. [#11](https://github.com/koppor/github-contributors-list/issues/11) - Fixed progressbar going backwards when using defaults. +### Changed + +- Handling of logins during ignoring users. + ## [2024-04-25] ### Added diff --git a/gcl.java b/gcl.java index 28c827f..852db6e 100755 --- a/gcl.java +++ b/gcl.java @@ -94,6 +94,7 @@ public class gcl implements Callable { @Option(names = "--filter") private List ignoredUsers = List.of( + "allcontributors[bot]", "dependabot[bot]", "dependabot", "apps/dependabot", @@ -126,7 +127,7 @@ public class gcl implements Callable { private static final String githubUsersEmailSuffix = "@users.noreply.github.com"; - private record Contributor(String name, String url, String avatarUrl) implements Serializable { + private record Contributor(String name, String url, String avatarUrl, String commitName) implements Serializable { public String getUserId() { // Example: https://github.com/LoayGhreeb, then userId is LoeyGhreeb return url.substring(url.lastIndexOf('/') + 1); @@ -324,21 +325,21 @@ private void analyzeCommit(ProgressBar progressBar, GitHub gitHub, GHRepository Logger.trace("Checking commit \"{}\" ({})", shortMessage, commit.getId().name()); Matcher matcher = numberAtEnd.matcher(shortMessage); - String number = null; + String prNumber = null; if (matcher.matches()) { - number = matcher.group(1); + prNumber = matcher.group(1); } - if (number == null) { + if (prNumber == null) { matcher = mergeCommit.matcher(shortMessage); if (matcher.matches()) { - number = matcher.group(1); + prNumber = matcher.group(1); } } - if (number == null) { + if (prNumber == null) { Logger.trace("No PR number found in commit message"); addContributorFromRevCommit(commit, emailToContributor, gitHub); } else { - if (!analyzePullRequest(progressBar, ghRepository, loginToContributor, emailToContributor, gitHub, number, commit.getName())) { + if (!analyzePullRequest(progressBar, ghRepository, loginToContributor, emailToContributor, gitHub, prNumber, commit.getName())) { Logger.trace("PR was 404. Interpreting commit itself"); addContributorFromRevCommit(commit, emailToContributor, gitHub); } @@ -347,10 +348,11 @@ private void analyzeCommit(ProgressBar progressBar, GitHub gitHub, GHRepository private void addContributorFromRevCommit(RevCommit commit, MVMap emailToContributor, GitHub gitHub) { CoAuthor authorOfCommit = new CoAuthor(commit.getAuthorIdent().getName(), commit.getAuthorIdent().getEmailAddress()); - analyzeRegularCommit(authorOfCommit, emailToContributor, gitHub, commit.getFullMessage()); + analyzeRegularCommit(authorOfCommit, emailToContributor, gitHub, commit.getName(), commit.getFullMessage()); } /** + * @param number the PR number * @return false if the PR did not exist */ private boolean analyzePullRequest(ProgressBar progressBar, GHRepository ghRepository, MVMap loginToContributor, MVMap emailToContributor, GitHub gitHub, String number, String prCommitNumber) throws IOException { @@ -362,11 +364,11 @@ private boolean analyzePullRequest(ProgressBar progressBar, GHRepository ghRepos try { pullRequest = ghRepository.getPullRequest(prNumber); } catch (GHFileNotFoundException e) { - Logger.warn("Pull request {} not found. Referenced from commit {}.", prNumber, prCommitNumber); + Logger.warn("Pull request #{} not found. Referenced from commit {}.", prNumber, prCommitNumber); return false; } GHUser user = pullRequest.getUser(); - storeContributorData(loginToContributor, emailToContributor, user); + storeContributorData(loginToContributor, emailToContributor, user, prCommitNumber); PagedIterator ghCommitIterator = pullRequest.listCommits().iterator(); while (ghCommitIterator.hasNext()) { @@ -381,14 +383,14 @@ private boolean analyzePullRequest(ProgressBar progressBar, GHRepository ghRepos // storeContributorData(loginToContributor, gitHub, theCommit.getCommitter().getUsername()); CoAuthor authorOfCommit = new CoAuthor(theCommit.getAuthor().getName(), theCommit.getAuthor().getEmail()); - analyzeRegularCommit(authorOfCommit, emailToContributor, gitHub, theCommit.getMessage()); + analyzeRegularCommit(authorOfCommit, emailToContributor, gitHub, ghCommit.getSha(), theCommit.getMessage()); } return true; } - private void analyzeRegularCommit(CoAuthor authorOfCommit, MVMap emailToContributor, GitHub gitHub, String commitMessage) { - Optional contributor = lookupContributorData(emailToContributor, gitHub, authorOfCommit); + private void analyzeRegularCommit(CoAuthor authorOfCommit, MVMap emailToContributor, GitHub gitHub, String commitName, String commitMessage) { + Optional contributor = lookupContributorData(emailToContributor, gitHub, commitName, authorOfCommit); // In case an author is ignored, the Optional is empty contributor.ifPresent(contributors::add); @@ -396,7 +398,7 @@ private void analyzeRegularCommit(CoAuthor authorOfCommit, MVMap line.startsWith("Co-authored-by:")) .map(CoAuthor::new) - .map(coAuthor -> lookupContributorData(emailToContributor, gitHub, coAuthor)) + .map(coAuthor -> lookupContributorData(emailToContributor, gitHub, commitName, coAuthor)) .filter(Optional::isPresent) .map(Optional::get) .forEach(contributors::add); @@ -453,17 +455,18 @@ private static String getFormattedSecondLine(Contributor contributor) { /** * Derives the contributor based on given ghUser and adds it to the loginToContributor and emailToContributor maps as well as to the contributors set. */ - private void storeContributorData(MVMap loginToContributor, MVMap emailToContributor, GHUser ghUser) { + private void storeContributorData(MVMap loginToContributor, MVMap emailToContributor, GHUser ghUser, String prCommitNumber) { Logger.trace("Handling {}", ghUser); String login = ghUser.getLogin(); + if (ignoredUsers.contains(login)) { + Logger.trace("Ignored because of login {}", login); + return; + } + Contributor contributor = loginToContributor.get(login); Logger.trace("Found contributor {}", contributor); if (contributor != null) { - if (ignoredUsers.contains(login)) { - Logger.trace("Ignored because of login {}", login); - return; - } contributors.add(contributor); putIntoEmailToContributorMap(emailToContributor, ghUser, contributor); return; @@ -485,7 +488,7 @@ private void storeContributorData(MVMap loginToContributor, return; } - Contributor newContributor = new Contributor(name, ghUser.getHtmlUrl().toString(), ghUser.getAvatarUrl()); + Contributor newContributor = new Contributor(name, ghUser.getHtmlUrl().toString(), ghUser.getAvatarUrl(), prCommitNumber); Logger.trace("Created new contributor {} based on PR data", newContributor); loginToContributor.put(login, newContributor); contributors.add(newContributor); @@ -509,7 +512,7 @@ private static void putIntoEmailToContributorMap(MVMap emai emailToContributor.put(email, contributor); } - private Optional lookupContributorData(MVMap emailToContributor, GitHub gitHub, CoAuthor coAuthor) { + private Optional lookupContributorData(MVMap emailToContributor, GitHub gitHub, String commitName, CoAuthor coAuthor) { Logger.trace("Looking up {}", coAuthor); if (alreadyChecked.contains(coAuthor)) { Logger.trace("Already checked {}", coAuthor); @@ -549,7 +552,7 @@ private Optional lookupContributorData(MVMap e Logger.trace("Online lookup disabled. Using {} as fallback.", coAuthor.name); fallbacks.add(coAuthor.name); fallbackSources.put(coAuthor.name, new PRAppearance(currentPR, currentSHA)); - return Optional.of(new Contributor(coAuthor.name, "", "")); + return Optional.of(new Contributor(coAuthor.name, "", "", currentSHA)); } PagedSearchIterable list = gitHub.searchUsers().q(email).list(); if (list.getTotalCount() == 1) { @@ -559,7 +562,7 @@ private Optional lookupContributorData(MVMap e Logger.trace("Ignored because of login {}: {}", login, coAuthor); return Optional.empty(); } - Contributor newContributor = new Contributor(login, user.getHtmlUrl().toString(), user.getAvatarUrl()); + Contributor newContributor = new Contributor(login, user.getHtmlUrl().toString(), user.getAvatarUrl(), commitName); emailToContributor.put(email, newContributor); return Optional.of(newContributor); } @@ -615,7 +618,7 @@ private Optional lookupContributorData(MVMap e Logger.trace("No user found for {}. Using {} as fallback.", coAuthor, coAuthor.name); fallbacks.add(coAuthor.name); fallbackSources.put(coAuthor.name, new PRAppearance(currentPR, currentSHA)); - return Optional.of(new Contributor(coAuthor.name, "", "")); + return Optional.of(new Contributor(coAuthor.name, "", "", currentSHA)); } String login = user.getLogin(); @@ -636,7 +639,7 @@ private Optional lookupContributorData(MVMap e name = usersLogin; } - Contributor newContributor = new Contributor(name, user.getHtmlUrl().toString(), user.getAvatarUrl()); + Contributor newContributor = new Contributor(name, user.getHtmlUrl().toString(), user.getAvatarUrl(), commitName); Logger.trace("Found user {} for {}", newContributor, coAuthor); From 6b1fd223c04e25584324a180b5f5a4cc56c4450a Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Fri, 26 Apr 2024 16:26:43 +0200 Subject: [PATCH 08/12] Better handling for NO_PR --- gcl.java | 41 +++++++++++++++++++++-------------------- 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/gcl.java b/gcl.java index 852db6e..17a2a74 100755 --- a/gcl.java +++ b/gcl.java @@ -65,7 +65,7 @@ import picocli.CommandLine.Parameters; @Command(name = "gcl", - version = "gcl 0.1.0", + version = "gcl 2024-04-26", mixinStandardHelpOptions = true, sortSynopsis = false) public class gcl implements Callable { @@ -76,6 +76,7 @@ public class gcl implements Callable { private static final Pattern numberAtEnd = Pattern.compile(".*\\(#(\\d+)\\)$"); private static final Pattern mergeCommit = Pattern.compile("^Merge pull request #(\\d+) from.*"); + private final String NO_PR = "(none)"; @Parameters(index = "0", arity = "0..1", description = "The path to the git repository to analyse.") private Path repositoryPath = Path.of("."); @@ -151,8 +152,6 @@ public CoAuthor(String line) { private record PRAppearance(String prNumber, String sha) { } - private String currentPR = ""; - private String currentSHA = ""; private MutableMultimap fallbackSources = Multimaps.mutable.sortedBag.empty(Comparator.comparing(PRAppearance::prNumber)); public static void main(String... args) throws Exception { @@ -302,10 +301,15 @@ private void outputFallbacks() { fallbacks.stream().forEach(fallback -> { String fallbackFormatted = String.format("%-" + maxLength + "s", fallback); fallbackSources.get(fallback).stream().forEach(pr -> { - System.out.println(fallbackFormatted - + " https://github.com/%s/pull/%s".formatted(ownerRepository, pr.prNumber) - + " https://github.com/%s/pull/%s/commits/%s".formatted(ownerRepository, pr.prNumber, pr.sha) - ); + if (pr.prNumber.equals(NO_PR)) { + System.out.println(fallbackFormatted + + " https://github.com/%s/commit/%s".formatted(ownerRepository, pr.sha)); + } else { + System.out.println(fallbackFormatted + + " https://github.com/%s/pull/%s".formatted(ownerRepository, pr.prNumber) + + " https://github.com/%s/pull/%s/commits/%s".formatted(ownerRepository, pr.prNumber, pr.sha) + ); + } }); }); } @@ -348,7 +352,7 @@ private void analyzeCommit(ProgressBar progressBar, GitHub gitHub, GHRepository private void addContributorFromRevCommit(RevCommit commit, MVMap emailToContributor, GitHub gitHub) { CoAuthor authorOfCommit = new CoAuthor(commit.getAuthorIdent().getName(), commit.getAuthorIdent().getEmailAddress()); - analyzeRegularCommit(authorOfCommit, emailToContributor, gitHub, commit.getName(), commit.getFullMessage()); + analyzeRegularCommit(authorOfCommit, emailToContributor, gitHub, NO_PR, commit.getName(), commit.getFullMessage()); } /** @@ -357,7 +361,6 @@ private void addContributorFromRevCommit(RevCommit commit, MVMap loginToContributor, MVMap emailToContributor, GitHub gitHub, String number, String prCommitNumber) throws IOException { Logger.trace("Investigating PR #{}", number); - this.currentPR = number; progressBar.setExtraMessage("PR " + number); int prNumber = Integer.parseInt(number); GHPullRequest pullRequest; @@ -375,22 +378,20 @@ private boolean analyzePullRequest(ProgressBar progressBar, GHRepository ghRepos GHPullRequestCommitDetail ghCommit = ghCommitIterator.next(); GHPullRequestCommitDetail.Commit theCommit = ghCommit.getCommit(); - this.currentSHA = ghCommit.getSha(); - // GitHub's API does not set the real GitHub username, so following does not work // This is very different from the information, which is available in GitHub's web interface // storeContributorData(loginToContributor, gitHub, theCommit.getAuthor().getUsername()); // storeContributorData(loginToContributor, gitHub, theCommit.getCommitter().getUsername()); CoAuthor authorOfCommit = new CoAuthor(theCommit.getAuthor().getName(), theCommit.getAuthor().getEmail()); - analyzeRegularCommit(authorOfCommit, emailToContributor, gitHub, ghCommit.getSha(), theCommit.getMessage()); + analyzeRegularCommit(authorOfCommit, emailToContributor, gitHub, number, ghCommit.getSha(), theCommit.getMessage()); } return true; } - private void analyzeRegularCommit(CoAuthor authorOfCommit, MVMap emailToContributor, GitHub gitHub, String commitName, String commitMessage) { - Optional contributor = lookupContributorData(emailToContributor, gitHub, commitName, authorOfCommit); + private void analyzeRegularCommit(CoAuthor authorOfCommit, MVMap emailToContributor, GitHub gitHub, String prNumber, String commitName, String commitMessage) { + Optional contributor = lookupContributorData(emailToContributor, gitHub, prNumber, commitName, authorOfCommit); // In case an author is ignored, the Optional is empty contributor.ifPresent(contributors::add); @@ -398,7 +399,7 @@ private void analyzeRegularCommit(CoAuthor authorOfCommit, MVMap line.startsWith("Co-authored-by:")) .map(CoAuthor::new) - .map(coAuthor -> lookupContributorData(emailToContributor, gitHub, commitName, coAuthor)) + .map(coAuthor -> lookupContributorData(emailToContributor, gitHub, prNumber, commitName, coAuthor)) .filter(Optional::isPresent) .map(Optional::get) .forEach(contributors::add); @@ -512,7 +513,7 @@ private static void putIntoEmailToContributorMap(MVMap emai emailToContributor.put(email, contributor); } - private Optional lookupContributorData(MVMap emailToContributor, GitHub gitHub, String commitName, CoAuthor coAuthor) { + private Optional lookupContributorData(MVMap emailToContributor, GitHub gitHub, String prNumber, String commitName, CoAuthor coAuthor) { Logger.trace("Looking up {}", coAuthor); if (alreadyChecked.contains(coAuthor)) { Logger.trace("Already checked {}", coAuthor); @@ -551,8 +552,8 @@ private Optional lookupContributorData(MVMap e if (!ghLookup) { Logger.trace("Online lookup disabled. Using {} as fallback.", coAuthor.name); fallbacks.add(coAuthor.name); - fallbackSources.put(coAuthor.name, new PRAppearance(currentPR, currentSHA)); - return Optional.of(new Contributor(coAuthor.name, "", "", currentSHA)); + fallbackSources.put(coAuthor.name, new PRAppearance(prNumber, commitName)); + return Optional.of(new Contributor(coAuthor.name, "", "", commitName)); } PagedSearchIterable list = gitHub.searchUsers().q(email).list(); if (list.getTotalCount() == 1) { @@ -617,8 +618,8 @@ private Optional lookupContributorData(MVMap e if (user == null) { Logger.trace("No user found for {}. Using {} as fallback.", coAuthor, coAuthor.name); fallbacks.add(coAuthor.name); - fallbackSources.put(coAuthor.name, new PRAppearance(currentPR, currentSHA)); - return Optional.of(new Contributor(coAuthor.name, "", "", currentSHA)); + fallbackSources.put(coAuthor.name, new PRAppearance(prNumber, commitName)); + return Optional.of(new Contributor(coAuthor.name, "", "", commitName)); } String login = user.getLogin(); From 70ecd9517a2deb2d08e95401db26090fd4c2a527 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Fri, 26 Apr 2024 16:49:08 +0200 Subject: [PATCH 09/12] Improved handling of commits in mainline --- CHANGELOG.md | 4 ++++ gcl.java | 52 ++++++++++++++++++++++++++++++++++++++++------------ 2 files changed, 44 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 11c1ee0..8ecf59b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ## [2024-04-26] +### Added + +- Improved handling of commits in mainline + ### Fixed - Fixed handling of 404 pull requests. [#11](https://github.com/koppor/github-contributors-list/issues/11) diff --git a/gcl.java b/gcl.java index 17a2a74..738dbad 100755 --- a/gcl.java +++ b/gcl.java @@ -50,6 +50,7 @@ import org.eclipse.jgit.revwalk.RevWalk; import org.h2.mvstore.MVMap; import org.h2.mvstore.MVStore; +import org.kohsuke.github.GHCommit; import org.kohsuke.github.GHFileNotFoundException; import org.kohsuke.github.GHPullRequest; import org.kohsuke.github.GHPullRequestCommitDetail; @@ -197,6 +198,7 @@ public Integer call() throws Exception { Logger.info("Connecting to {}...", ownerRepository); GitHub gitHub = GitHub.connect(); + GHRepository gitHubRepository; try { gitHubRepository = gitHub.getRepository(ownerRepository); @@ -341,17 +343,39 @@ private void analyzeCommit(ProgressBar progressBar, GitHub gitHub, GHRepository } if (prNumber == null) { Logger.trace("No PR number found in commit message"); - addContributorFromRevCommit(commit, emailToContributor, gitHub); + addContributorFromRevCommit(commit, emailToContributor, gitHub, ghRepository); } else { if (!analyzePullRequest(progressBar, ghRepository, loginToContributor, emailToContributor, gitHub, prNumber, commit.getName())) { Logger.trace("PR was 404. Interpreting commit itself"); - addContributorFromRevCommit(commit, emailToContributor, gitHub); + addContributorFromRevCommit(commit, emailToContributor, gitHub, ghRepository); } } } - private void addContributorFromRevCommit(RevCommit commit, MVMap emailToContributor, GitHub gitHub) { + private void addContributorFromRevCommit(RevCommit commit, MVMap emailToContributor, GitHub gitHub, GHRepository ghRepository) { CoAuthor authorOfCommit = new CoAuthor(commit.getAuthorIdent().getName(), commit.getAuthorIdent().getEmailAddress()); + GHCommit commit1 = null; + try { + commit1 = ghRepository.getCommit(commit.getName()); + } catch (IOException e) { + Logger.error("Could not get commit {} from GitHub", commit.getName(), e); + } + + if ((commit1 != null)) { + try { + GHUser author = commit1.getAuthor(); + if (author == null) { + Logger.debug("GitHub did not provide author for {} ({})", commit.getName(), commit.getAuthorIdent().toExternalString()); + } else { + String login = author.getLogin(); + Contributor contributor = new Contributor(login, author.getHtmlUrl().toString(), author.getAvatarUrl(), commit.getName()); + emailToContributor.put(commit.getAuthorIdent().getEmailAddress(), contributor); + } + } catch (IOException e) { + Logger.error("Could not get login information from commit {} from GitHub", commit.getName(), e); + } + } + analyzeRegularCommit(authorOfCommit, emailToContributor, gitHub, NO_PR, commit.getName(), commit.getFullMessage()); } @@ -376,20 +400,24 @@ private boolean analyzePullRequest(ProgressBar progressBar, GHRepository ghRepos PagedIterator ghCommitIterator = pullRequest.listCommits().iterator(); while (ghCommitIterator.hasNext()) { GHPullRequestCommitDetail ghCommit = ghCommitIterator.next(); - GHPullRequestCommitDetail.Commit theCommit = ghCommit.getCommit(); - - // GitHub's API does not set the real GitHub username, so following does not work - // This is very different from the information, which is available in GitHub's web interface - // storeContributorData(loginToContributor, gitHub, theCommit.getAuthor().getUsername()); - // storeContributorData(loginToContributor, gitHub, theCommit.getCommitter().getUsername()); - - CoAuthor authorOfCommit = new CoAuthor(theCommit.getAuthor().getName(), theCommit.getAuthor().getEmail()); - analyzeRegularCommit(authorOfCommit, emailToContributor, gitHub, number, ghCommit.getSha(), theCommit.getMessage()); + analyzeGhCommit(emailToContributor, gitHub, number, ghCommit); } return true; } + private void analyzeGhCommit(MVMap emailToContributor, GitHub gitHub, String number, GHPullRequestCommitDetail ghCommit) { + GHPullRequestCommitDetail.Commit theCommit = ghCommit.getCommit(); + + // GitHub's API does not set the real GitHub username, so following does not work + // This is very different from the information, which is available in GitHub's web interface + // storeContributorData(loginToContributor, gitHub, theCommit.getAuthor().getUsername()); + // storeContributorData(loginToContributor, gitHub, theCommit.getCommitter().getUsername()); + + CoAuthor authorOfCommit = new CoAuthor(theCommit.getAuthor().getName(), theCommit.getAuthor().getEmail()); + analyzeRegularCommit(authorOfCommit, emailToContributor, gitHub, number, ghCommit.getSha(), theCommit.getMessage()); + } + private void analyzeRegularCommit(CoAuthor authorOfCommit, MVMap emailToContributor, GitHub gitHub, String prNumber, String commitName, String commitMessage) { Optional contributor = lookupContributorData(emailToContributor, gitHub, prNumber, commitName, authorOfCommit); // In case an author is ignored, the Optional is empty From a0c45ff2868e831176ba19c0b6bf22859689d9a8 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Fri, 26 Apr 2024 16:52:32 +0200 Subject: [PATCH 10/12] Fix linter warnings --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8ecf59b..3bca192 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -48,3 +48,5 @@ Initial release. [2024-04-26]: https://github.com/koppor/github-contributors-list/compare/2024-04-25...2024-04-26 [2024-04-25]: https://github.com/koppor/github-contributors-list/compare/2024-04-09...2024-04-25 [2024-04-09]: https://github.com/koppor/github-contributors-list/releases/tag/2024-04-09 + + From ba74bbe5841356aa54bab1f3e3bad7955c4ebf35 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Fri, 26 Apr 2024 17:11:14 +0200 Subject: [PATCH 11/12] More linting fixes --- .github/workflows/ci.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index dce2110..433b671 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -49,4 +49,4 @@ jobs: # exit 1 in case of error # We have 1 "valid" issue in CHANGELOG.md - grep -q "2 problems" heylogs.txt || exit 1 + grep -q "3 problems" heylogs.txt || exit 1 From a2d84e926860b798fd5e2cfa3d4de4c0462f7492 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Fri, 26 Apr 2024 17:14:34 +0200 Subject: [PATCH 12/12] Fix duplicate issues --- gcl.java | 48 ++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 44 insertions(+), 4 deletions(-) diff --git a/gcl.java b/gcl.java index 738dbad..390581d 100755 --- a/gcl.java +++ b/gcl.java @@ -26,6 +26,7 @@ import java.util.Comparator; import java.util.HashSet; import java.util.Iterator; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Optional; @@ -367,9 +368,30 @@ private void addContributorFromRevCommit(RevCommit commit, MVMap { boolean isShorterList = subList.size() < cols; @@ -465,6 +487,24 @@ private void printMarkdownSnippet() { }); } + private List cleanUpContributors(SortedSet contributors) { + LinkedHashMap urlToContributor = new LinkedHashMap<>(); + contributors.forEach(contributor -> { + if (urlToContributor.containsKey(contributor.url())) { + Logger.trace("Duplicate URL found: {}", contributor.url()); + Contributor existingContributor = urlToContributor.get(contributor.url()); + if (!existingContributor.name().contains(" ")) { + // Heuristics: Names without space are not real names (but rather login names) + Logger.trace("Replacing {} with {}", existingContributor, contributor); + urlToContributor.put(contributor.url(), contributor); + } + } else { + urlToContributor.put(contributor.url(), contributor); + } + }); + return urlToContributor.values().stream().toList(); + } + private static String getFormattedFirstLine(Contributor contributor) { if (contributor.url().isEmpty()) { return contributor.name();