Skip to content

Commit a16c324

Browse files
Thomas StrombergThomas Stromberg
Thomas Stromberg
authored and
Thomas Stromberg
committed
simplify
1 parent 8668251 commit a16c324

File tree

5 files changed

+80
-61
lines changed

5 files changed

+80
-61
lines changed

‎cmd/prcost/main.go‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"flag"
99
"fmt"
1010
"log"
11+
"log/slog"
1112
"os"
1213
"os/exec"
1314
"strings"
@@ -18,6 +19,12 @@ import (
1819
)
1920

2021
func main() {
22+
// Setup structured logging to stderr (stdout is for results)
23+
logger := slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{
24+
Level: slog.LevelInfo,
25+
}))
26+
slog.SetDefault(logger)
27+
2128
// Define command-line flags
2229
salary := flag.Float64("salary", 250000, "Annual salary for cost calculation")
2330
benefits := flag.Float64("benefits", 1.3, "Benefits multiplier (1.3 = 30% benefits)")
@@ -53,28 +60,47 @@ func main() {
5360
log.Fatal("Invalid PR URL. Expected format: https://github.com/owner/repo/pull/123")
5461
}
5562

63+
slog.Info("Starting PR cost analysis", "pr_url", prURL, "format", *format)
64+
5665
// Create cost configuration from flags
5766
cfg := cost.DefaultConfig()
5867
cfg.AnnualSalary = *salary
5968
cfg.BenefitsMultiplier = *benefits
6069
cfg.EventDuration = time.Duration(*eventMinutes) * time.Minute
6170
cfg.DelayCostFactor = *overheadFactor
6271

72+
slog.Debug("Configuration",
73+
"salary", cfg.AnnualSalary,
74+
"benefits_multiplier", cfg.BenefitsMultiplier,
75+
"event_minutes", *eventMinutes,
76+
"delay_cost_factor", cfg.DelayCostFactor)
77+
6378
// Retrieve GitHub token from gh CLI
6479
ctx := context.Background()
80+
slog.Info("Retrieving GitHub authentication token")
6581
token, err := authToken(ctx)
6682
if err != nil {
83+
slog.Error("Failed to get GitHub token", "error", err)
6784
log.Fatalf("Failed to get GitHub token: %v\nPlease ensure 'gh' is installed and authenticated (run 'gh auth login')", err)
6885
}
86+
slog.Debug("Successfully retrieved GitHub token")
6987

7088
// Fetch PR data
89+
slog.Info("Fetching PR data from GitHub")
7190
prData, err := github.FetchPRData(ctx, prURL, token)
7291
if err != nil {
92+
slog.Error("Failed to fetch PR data", "error", err)
7393
log.Fatalf("Failed to fetch PR data: %v", err)
7494
}
95+
slog.Info("Successfully fetched PR data",
96+
"lines_added", prData.LinesAdded,
97+
"author", prData.Author,
98+
"events", len(prData.Events))
7599

76100
// Calculate costs
101+
slog.Info("Calculating PR costs")
77102
breakdown := cost.Calculate(prData, cfg)
103+
slog.Info("Cost calculation complete", "total_cost", breakdown.TotalCost)
78104

79105
// Output in requested format
80106
switch *format {

‎pkg/cost/cost.go‎

Lines changed: 21 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -64,13 +64,6 @@ func DefaultConfig() Config {
6464
}
6565
}
6666

67-
// HourlyRate calculates the hourly rate from annual salary including benefits.
68-
// Formula: (salary × benefits_multiplier) / hours_per_year.
69-
func (c Config) HourlyRate() float64 {
70-
totalCompensation := c.AnnualSalary * c.BenefitsMultiplier
71-
return totalCompensation / c.HoursPerYear
72-
}
73-
7467
// ParticipantEvent represents a single event by a participant.
7568
type ParticipantEvent struct {
7669
Timestamp time.Time
@@ -172,7 +165,11 @@ type Breakdown struct {
172165

173166
// Calculate computes the total cost of a pull request with detailed breakdowns.
174167
func Calculate(data PRData, cfg Config) Breakdown {
175-
hourlyRate := cfg.HourlyRate()
168+
// Defensive check: avoid division by zero
169+
if cfg.HoursPerYear == 0 {
170+
cfg.HoursPerYear = 2080 // Standard full-time hours per year
171+
}
172+
hourlyRate := (cfg.AnnualSalary * cfg.BenefitsMultiplier) / cfg.HoursPerYear
176173

177174
// Calculate author costs
178175
authorCost := calculateAuthorCost(data, cfg, hourlyRate)
@@ -182,6 +179,10 @@ func Calculate(data PRData, cfg Config) Breakdown {
182179

183180
// Calculate delay cost with itemized breakdown (always shown)
184181
delayHours := data.UpdatedAt.Sub(data.CreatedAt).Hours()
182+
// Defensive check: if UpdatedAt is before CreatedAt (bad data), treat as zero delay
183+
if delayHours < 0 {
184+
delayHours = 0
185+
}
185186
delayDays := delayHours / 24.0
186187

187188
// Cap Project Delay at configured maximum (default: 60 days / 2 months)
@@ -327,7 +328,12 @@ func calculateAuthorCost(data PRData, cfg Config, hourlyRate float64) AuthorCost
327328
codeContextCost := codeContextHours * hourlyRate
328329

329330
// 3. GitHub Cost + GitHub Context Cost: Based on author's events
330-
authorEvents := filterEventsByActor(data.Events, data.Author)
331+
var authorEvents []ParticipantEvent
332+
for _, event := range data.Events {
333+
if event.Actor == data.Author {
334+
authorEvents = append(authorEvents, event)
335+
}
336+
}
331337
githubHours, githubContextHours, sessions := calculateSessionCosts(authorEvents, cfg)
332338
githubCost := githubHours * hourlyRate
333339
githubContextCost := githubContextHours * hourlyRate
@@ -355,7 +361,12 @@ func calculateAuthorCost(data PRData, cfg Config, hourlyRate float64) AuthorCost
355361
// calculateParticipantCosts computes costs for all participants except the author.
356362
func calculateParticipantCosts(data PRData, cfg Config, hourlyRate float64) []ParticipantCostDetail {
357363
// Group events by actor
358-
eventsByActor := groupEventsByActor(data.Events, data.Author)
364+
eventsByActor := make(map[string][]ParticipantEvent)
365+
for _, event := range data.Events {
366+
if event.Actor != data.Author {
367+
eventsByActor[event.Actor] = append(eventsByActor[event.Actor], event)
368+
}
369+
}
359370

360371
var participantCosts []ParticipantCostDetail
361372

@@ -469,25 +480,3 @@ func calculateSessionCosts(events []ParticipantEvent, cfg Config) (githubHours,
469480

470481
return githubHours, contextHours, sessions
471482
}
472-
473-
// filterEventsByActor returns events for a specific actor.
474-
func filterEventsByActor(events []ParticipantEvent, actor string) []ParticipantEvent {
475-
var filtered []ParticipantEvent
476-
for _, event := range events {
477-
if event.Actor == actor {
478-
filtered = append(filtered, event)
479-
}
480-
}
481-
return filtered
482-
}
483-
484-
// groupEventsByActor groups events by actor, excluding the specified author.
485-
func groupEventsByActor(events []ParticipantEvent, excludeAuthor string) map[string][]ParticipantEvent {
486-
grouped := make(map[string][]ParticipantEvent)
487-
for _, event := range events {
488-
if event.Actor != excludeAuthor {
489-
grouped[event.Actor] = append(grouped[event.Actor], event)
490-
}
491-
}
492-
return grouped
493-
}

‎pkg/cost/cost_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ func TestDefaultConfig(t *testing.T) {
4242

4343
func TestHourlyRate(t *testing.T) {
4444
cfg := DefaultConfig()
45-
hourlyRate := cfg.HourlyRate()
45+
hourlyRate := (cfg.AnnualSalary * cfg.BenefitsMultiplier) / cfg.HoursPerYear
4646

4747
// $250,000 * 1.3 / 2080 = $156.25/hr
4848
expectedRate := 156.25

‎pkg/github/fetch.go‎

Lines changed: 25 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"context"
66
"errors"
77
"fmt"
8+
"log/slog"
89
"os"
910
"strconv"
1011
"strings"
@@ -54,60 +55,63 @@ func PRDataFromPRX(prData *prx.PullRequestData) cost.PRData {
5455
// - cost.PRData with all information needed for cost calculation
5556
func FetchPRData(ctx context.Context, prURL string, token string) (cost.PRData, error) {
5657
// Parse the PR URL to extract owner, repo, and PR number
57-
parts, err := parsePRURL(prURL)
58+
owner, repo, number, err := parsePRURL(prURL)
5859
if err != nil {
60+
slog.Error("Failed to parse PR URL", "url", prURL, "error", err)
5961
return cost.PRData{}, fmt.Errorf("invalid PR URL: %w", err)
6062
}
6163

64+
slog.Debug("Parsed PR URL", "owner", owner, "repo", repo, "number", number)
65+
6266
// Create prx client
6367
client := prx.NewClient(token)
6468

65-
// Fetch PR data using prx
66-
prData, err := client.PullRequest(ctx, parts.owner, parts.repo, parts.number)
69+
// Fetch PR data using prx (prx has built-in retry logic)
70+
slog.Debug("Calling GitHub API via prx", "owner", owner, "repo", repo, "pr", number)
71+
prData, err := client.PullRequest(ctx, owner, repo, number)
6772
if err != nil {
73+
slog.Error("GitHub API call failed", "owner", owner, "repo", repo, "pr", number, "error", err)
6874
return cost.PRData{}, fmt.Errorf("failed to fetch PR data: %w", err)
6975
}
7076

71-
// Convert to cost.PRData
72-
return PRDataFromPRX(prData), nil
73-
}
77+
slog.Debug("GitHub API call successful",
78+
"additions", prData.PullRequest.Additions,
79+
"author", prData.PullRequest.Author,
80+
"total_events", len(prData.Events))
7481

75-
// prParts holds the parsed components of a GitHub PR URL.
76-
type prParts struct {
77-
owner string
78-
repo string
79-
number int
82+
// Convert to cost.PRData
83+
result := PRDataFromPRX(prData)
84+
slog.Debug("Converted PR data", "human_events", len(result.Events))
85+
return result, nil
8086
}
8187

8288
// parsePRURL extracts owner, repo, and PR number from a GitHub PR URL.
8389
// Expected format: https://github.com/owner/repo/pull/123
84-
func parsePRURL(prURL string) (prParts, error) {
90+
//
91+
//nolint:revive // Four return values is simpler than creating a struct wrapper
92+
func parsePRURL(prURL string) (owner, repo string, number int, err error) {
8593
// Remove protocol prefix
8694
prURL = strings.TrimPrefix(prURL, "https://")
8795
prURL = strings.TrimPrefix(prURL, "http://")
8896

8997
// Remove github.com prefix
9098
if !strings.HasPrefix(prURL, "github.com/") {
91-
return prParts{}, errors.New("URL must be from github.com")
99+
return "", "", 0, errors.New("URL must be from github.com")
92100
}
93101
prURL = strings.TrimPrefix(prURL, "github.com/")
94102

95103
// Split by /
96104
parts := strings.Split(prURL, "/")
97105
if len(parts) < 4 || parts[2] != "pull" {
98-
return prParts{}, errors.New("expected format: https://github.com/owner/repo/pull/123")
106+
return "", "", 0, errors.New("expected format: https://github.com/owner/repo/pull/123")
99107
}
100108

101-
number, err := strconv.Atoi(parts[3])
109+
number, err = strconv.Atoi(parts[3])
102110
if err != nil {
103-
return prParts{}, fmt.Errorf("invalid PR number: %w", err)
111+
return "", "", 0, fmt.Errorf("invalid PR number: %w", err)
104112
}
105113

106-
return prParts{
107-
owner: parts[0],
108-
repo: parts[1],
109-
number: number,
110-
}, nil
114+
return parts[0], parts[1], number, nil
111115
}
112116

113117
// FetchPRDataWithDefaults is a convenience function that uses environment variables

‎pkg/github/fetch_test.go‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -66,22 +66,22 @@ func TestParsePRURL(t *testing.T) {
6666

6767
for _, tt := range tests {
6868
t.Run(tt.name, func(t *testing.T) {
69-
parts, err := parsePRURL(tt.url)
69+
owner, repo, number, err := parsePRURL(tt.url)
7070

7171
if (err != nil) != tt.wantErr {
7272
t.Errorf("parsePRURL() error = %v, wantErr %v", err, tt.wantErr)
7373
return
7474
}
7575

7676
if !tt.wantErr {
77-
if parts.owner != tt.wantOwner {
78-
t.Errorf("parsePRURL() owner = %v, want %v", parts.owner, tt.wantOwner)
77+
if owner != tt.wantOwner {
78+
t.Errorf("parsePRURL() owner = %v, want %v", owner, tt.wantOwner)
7979
}
80-
if parts.repo != tt.wantRepo {
81-
t.Errorf("parsePRURL() repo = %v, want %v", parts.repo, tt.wantRepo)
80+
if repo != tt.wantRepo {
81+
t.Errorf("parsePRURL() repo = %v, want %v", repo, tt.wantRepo)
8282
}
83-
if parts.number != tt.wantNumber {
84-
t.Errorf("parsePRURL() number = %v, want %v", parts.number, tt.wantNumber)
83+
if number != tt.wantNumber {
84+
t.Errorf("parsePRURL() number = %v, want %v", number, tt.wantNumber)
8585
}
8686
}
8787
})

0 commit comments

Comments
 (0)