mirror of
https://github.com/danielmiessler/Fabric.git
synced 2026-01-10 06:48:04 -05:00
feat: improve timestamp handling and merge commit detection in changelog generator
## CHANGES - Add debug logging for date parsing failures - Pass forcePRSync parameter explicitly to fetchPRs method - Implement comprehensive merge commit detection using parents - Capture actual commit timestamps from GitHub API - Calculate version dates from most recent commit - Add parent commit SHAs for merge detection - Use real commit dates instead of current time - Add timestamp validation with fallback handling
This commit is contained in:
@@ -204,7 +204,11 @@ func (c *Cache) GetVersions() (map[string]*git.Version, error) {
|
||||
// Try RFC3339Nano first (for nanosecond precision), then fall back to RFC3339
|
||||
v.Date, err = time.Parse(time.RFC3339Nano, dateStr.String)
|
||||
if err != nil {
|
||||
v.Date, _ = time.Parse(time.RFC3339, dateStr.String)
|
||||
fmt.Printf("Error parsing date with RFC3339Nano: %v\n", err)
|
||||
v.Date, err = time.Parse(time.RFC3339, dateStr.String)
|
||||
if err != nil {
|
||||
fmt.Printf("Error parsing date with RFC3339: %v\n", err)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -65,7 +65,7 @@ func (g *Generator) Generate() (string, error) {
|
||||
return "", fmt.Errorf("failed to collect data: %w", err)
|
||||
}
|
||||
|
||||
if err := g.fetchPRs(); err != nil {
|
||||
if err := g.fetchPRs(g.cfg.ForcePRSync); err != nil {
|
||||
return "", fmt.Errorf("failed to fetch PRs: %w", err)
|
||||
}
|
||||
|
||||
@@ -193,7 +193,7 @@ func (g *Generator) collectData() error {
|
||||
return nil
|
||||
}
|
||||
|
||||
func (g *Generator) fetchPRs() error {
|
||||
func (g *Generator) fetchPRs(forcePRSync bool) error {
|
||||
// First, load all cached PRs
|
||||
if g.cache != nil {
|
||||
cachedPRs, err := g.cache.GetAllPRs()
|
||||
@@ -229,7 +229,7 @@ func (g *Generator) fetchPRs() error {
|
||||
}
|
||||
// If we have never synced or it's been more than 24 hours, do a full sync
|
||||
// Also sync if we have versions with PR numbers that aren't cached
|
||||
needsSync := lastSync.IsZero() || time.Since(lastSync) > 24*time.Hour || g.cfg.ForcePRSync || missingPRs
|
||||
needsSync := lastSync.IsZero() || time.Since(lastSync) > 24*time.Hour || forcePRSync || missingPRs
|
||||
|
||||
if !needsSync {
|
||||
fmt.Fprintf(os.Stderr, "Using cached PR data (last sync: %s)\n", lastSync.Format("2006-01-02 15:04:05"))
|
||||
@@ -706,10 +706,9 @@ func (g *Generator) SyncDatabase() error {
|
||||
|
||||
fmt.Fprintf(os.Stderr, "🔄 Starting database synchronization...\n")
|
||||
|
||||
// Step 1: Force PR sync (reuse existing logic)
|
||||
// Step 1: Force PR sync (pass true explicitly)
|
||||
fmt.Fprintf(os.Stderr, "📥 Forcing PR sync from GitHub...\n")
|
||||
g.cfg.ForcePRSync = true
|
||||
if err := g.fetchPRs(); err != nil {
|
||||
if err := g.fetchPRs(true); err != nil {
|
||||
return fmt.Errorf("failed to sync PRs: %w", err)
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,82 @@
|
||||
package changelog
|
||||
|
||||
import (
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/danielmiessler/fabric/cmd/generate_changelog/internal/github"
|
||||
)
|
||||
|
||||
func TestIsMergeCommit(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
commit github.PRCommit
|
||||
expected bool
|
||||
}{
|
||||
{
|
||||
name: "Regular commit with single parent",
|
||||
commit: github.PRCommit{
|
||||
SHA: "abc123",
|
||||
Message: "Fix bug in user authentication",
|
||||
Author: "John Doe",
|
||||
Date: time.Now(),
|
||||
Parents: []string{"def456"},
|
||||
},
|
||||
expected: false,
|
||||
},
|
||||
{
|
||||
name: "Merge commit with multiple parents",
|
||||
commit: github.PRCommit{
|
||||
SHA: "abc123",
|
||||
Message: "Merge pull request #42 from feature/auth",
|
||||
Author: "GitHub",
|
||||
Date: time.Now(),
|
||||
Parents: []string{"def456", "ghi789"},
|
||||
},
|
||||
expected: true,
|
||||
},
|
||||
{
|
||||
name: "Merge commit detected by message pattern only",
|
||||
commit: github.PRCommit{
|
||||
SHA: "abc123",
|
||||
Message: "Merge pull request #123 from user/feature-branch",
|
||||
Author: "GitHub",
|
||||
Date: time.Now(),
|
||||
Parents: []string{}, // Empty parents - fallback to message detection
|
||||
},
|
||||
expected: true,
|
||||
},
|
||||
{
|
||||
name: "Merge branch commit pattern",
|
||||
commit: github.PRCommit{
|
||||
SHA: "abc123",
|
||||
Message: "Merge branch 'feature' into main",
|
||||
Author: "Developer",
|
||||
Date: time.Now(),
|
||||
Parents: []string{"def456"}, // Single parent but merge pattern
|
||||
},
|
||||
expected: true,
|
||||
},
|
||||
{
|
||||
name: "Regular commit with no merge patterns",
|
||||
commit: github.PRCommit{
|
||||
SHA: "abc123",
|
||||
Message: "Add new feature for user management",
|
||||
Author: "Jane Doe",
|
||||
Date: time.Now(),
|
||||
Parents: []string{"def456"},
|
||||
},
|
||||
expected: false,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
result := isMergeCommit(tt.commit)
|
||||
if result != tt.expected {
|
||||
t.Errorf("isMergeCommit() = %v, expected %v for commit: %s",
|
||||
result, tt.expected, tt.commit.Message)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -14,6 +14,32 @@ import (
|
||||
"github.com/danielmiessler/fabric/cmd/generate_changelog/internal/github"
|
||||
)
|
||||
|
||||
// isMergeCommit determines if a commit is a merge commit based on multiple criteria
|
||||
func isMergeCommit(commit github.PRCommit) bool {
|
||||
// Primary method: Check parent count (merge commits have multiple parents)
|
||||
if len(commit.Parents) > 1 {
|
||||
return true
|
||||
}
|
||||
|
||||
// Fallback method: Check commit message patterns
|
||||
// Common merge commit message patterns
|
||||
mergePatterns := []string{
|
||||
`^Merge pull request #\d+`, // "Merge pull request #123 from..."
|
||||
`^Merge branch '.*' into .*`, // "Merge branch 'feature' into main"
|
||||
`^Merge remote-tracking branch`, // "Merge remote-tracking branch..."
|
||||
`^Merge '.*' into .*`, // "Merge 'feature' into main"
|
||||
`^Merge .*`, // General "Merge ..." patterns
|
||||
}
|
||||
|
||||
for _, pattern := range mergePatterns {
|
||||
if matched, _ := regexp.MatchString(pattern, commit.Message); matched {
|
||||
return true
|
||||
}
|
||||
}
|
||||
|
||||
return false
|
||||
}
|
||||
|
||||
// ProcessIncomingPR processes a single PR for changelog entry creation
|
||||
func (g *Generator) ProcessIncomingPR(prNumber int) error {
|
||||
if err := g.validatePRState(prNumber); err != nil {
|
||||
@@ -129,8 +155,24 @@ func (g *Generator) CreateNewChangelogEntry(version string) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// Calculate the version date for the changelog entry as the most recent commit date from processed PRs
|
||||
changelogDate := time.Now() // fallback to current time
|
||||
if len(fetchedPRs) > 0 {
|
||||
var mostRecentCommitDate time.Time
|
||||
for _, pr := range fetchedPRs {
|
||||
for _, commit := range pr.Commits {
|
||||
if commit.Date.After(mostRecentCommitDate) {
|
||||
mostRecentCommitDate = commit.Date
|
||||
}
|
||||
}
|
||||
}
|
||||
if !mostRecentCommitDate.IsZero() {
|
||||
changelogDate = mostRecentCommitDate
|
||||
}
|
||||
}
|
||||
|
||||
entry := fmt.Sprintf("## %s (%s)\n\n%s",
|
||||
version, time.Now().Format("2006-01-02"), strings.TrimLeft(content.String(), "\n"))
|
||||
version, changelogDate.Format("2006-01-02"), strings.TrimLeft(content.String(), "\n"))
|
||||
|
||||
if err := g.insertVersionAtTop(entry); err != nil {
|
||||
return fmt.Errorf("failed to update CHANGELOG.md: %w", err)
|
||||
@@ -152,14 +194,21 @@ func (g *Generator) CreateNewChangelogEntry(version string) error {
|
||||
// Save individual commits to cache for each PR
|
||||
for _, pr := range fetchedPRs {
|
||||
for _, commit := range pr.Commits {
|
||||
// Use actual commit timestamp, with fallback to current time if invalid
|
||||
commitDate := commit.Date
|
||||
if commitDate.IsZero() {
|
||||
commitDate = time.Now()
|
||||
fmt.Fprintf(os.Stderr, "Warning: Commit %s has invalid timestamp, using current time\n", commit.SHA)
|
||||
}
|
||||
|
||||
// Convert github.PRCommit to git.Commit
|
||||
gitCommit := &git.Commit{
|
||||
SHA: commit.SHA,
|
||||
Message: commit.Message,
|
||||
Author: commit.Author,
|
||||
Email: "", // Not available from GitHub API
|
||||
Date: time.Now(), // Use current time as fallback
|
||||
IsMerge: false, // We don't have this info from GitHub API
|
||||
Email: "", // Not available from GitHub API
|
||||
Date: commitDate, // Use actual commit timestamp from GitHub API
|
||||
IsMerge: isMergeCommit(commit), // Detect merge commits using parents and message patterns
|
||||
PRNumber: pr.Number,
|
||||
}
|
||||
if err := g.cache.SaveCommit(gitCommit, version); err != nil {
|
||||
@@ -169,12 +218,28 @@ func (g *Generator) CreateNewChangelogEntry(version string) error {
|
||||
}
|
||||
}
|
||||
|
||||
// Calculate the version date as the most recent commit date from processed PRs
|
||||
versionDate := time.Now() // fallback to current time
|
||||
if len(fetchedPRs) > 0 {
|
||||
var mostRecentCommitDate time.Time
|
||||
for _, pr := range fetchedPRs {
|
||||
for _, commit := range pr.Commits {
|
||||
if commit.Date.After(mostRecentCommitDate) {
|
||||
mostRecentCommitDate = commit.Date
|
||||
}
|
||||
}
|
||||
}
|
||||
if !mostRecentCommitDate.IsZero() {
|
||||
versionDate = mostRecentCommitDate
|
||||
}
|
||||
}
|
||||
|
||||
// Create a proper new version entry for the database
|
||||
newVersionEntry := &git.Version{
|
||||
Name: version,
|
||||
Date: time.Now(),
|
||||
CommitSHA: "", // Will be set when the release commit is made
|
||||
PRNumbers: prNumbers, // Now we have the actual PR numbers
|
||||
Date: versionDate, // Use most recent commit date instead of current time
|
||||
CommitSHA: "", // Will be set when the release commit is made
|
||||
PRNumbers: prNumbers, // Now we have the actual PR numbers
|
||||
AISummary: content.String(),
|
||||
}
|
||||
|
||||
|
||||
@@ -207,6 +207,18 @@ func (c *Client) convertGitHubPR(ghPR *github.PullRequest, commits []*github.Rep
|
||||
}
|
||||
if commit.Commit.Author != nil {
|
||||
prCommit.Author = getString(commit.Commit.Author.Name)
|
||||
// Capture actual commit timestamp from GitHub API
|
||||
if commit.Commit.Author.Date != nil {
|
||||
prCommit.Date = commit.Commit.Author.Date.Time
|
||||
}
|
||||
}
|
||||
// Capture parent commit SHAs for merge detection
|
||||
if commit.Parents != nil {
|
||||
for _, parent := range commit.Parents {
|
||||
if parent.SHA != nil {
|
||||
prCommit.Parents = append(prCommit.Parents, *parent.SHA)
|
||||
}
|
||||
}
|
||||
}
|
||||
result.Commits = append(result.Commits, prCommit)
|
||||
}
|
||||
@@ -395,6 +407,7 @@ func (c *Client) FetchAllMergedPRsGraphQL(since time.Time) ([]*PR, error) {
|
||||
SHA: commitNode.Commit.OID,
|
||||
Message: strings.TrimSpace(commitNode.Commit.Message),
|
||||
Author: commitNode.Commit.Author.Name,
|
||||
Date: commitNode.Commit.AuthoredDate, // Use actual commit timestamp
|
||||
}
|
||||
pr.Commits = append(pr.Commits, commit)
|
||||
}
|
||||
|
||||
@@ -26,6 +26,8 @@ type PRCommit struct {
|
||||
SHA string
|
||||
Message string
|
||||
Author string
|
||||
Date time.Time // Add timestamp field
|
||||
Parents []string // Add parent commits for merge detection
|
||||
}
|
||||
|
||||
// GraphQL query structures for hasura client
|
||||
@@ -50,9 +52,10 @@ type PullRequestsQuery struct {
|
||||
Commits struct {
|
||||
Nodes []struct {
|
||||
Commit struct {
|
||||
OID string `graphql:"oid"`
|
||||
Message string
|
||||
Author struct {
|
||||
OID string `graphql:"oid"`
|
||||
Message string
|
||||
AuthoredDate time.Time `graphql:"authoredDate"`
|
||||
Author struct {
|
||||
Name string
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user