-
Notifications
You must be signed in to change notification settings - Fork 561
feat/alias for project names #2070
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
Conversation
motatoes
commented
Aug 6, 2025
- allow using of alias for projects
- support alias delimeter
- also update on comment side
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Greptile Summary
This PR implements a comprehensive project alias system throughout the Digger codebase. The changes introduce an Alias
field to the core Project
struct in digger_config
, allowing projects to have alternative, more user-friendly names alongside their internal names.
The implementation spans multiple layers of the system:
Configuration Layer: The Project
and ProjectYaml
structs now include an optional Alias
field with proper YAML serialization support (alias,omitempty
). For Terragrunt configurations, a configurable ProjectAliasDelimiter
field allows customization of how aliases are generated from directory names, with "_" as the default.
Job Scheduling: The Job
and SerializedJob
structs in the scheduler package now carry ProjectAlias
information with proper JSON serialization. A GetProjectAlias()
method provides fallback logic - returning the alias if present, otherwise the project name.
GitHub Integration: PR status checks, comments, and CI pipeline integrations now use job.GetProjectAlias()
instead of job.ProjectName
for display purposes. This affects status contexts like "project-alias/plan" and "project-alias/apply" in GitHub.
Terragrunt Support: The Terragrunt parsing pipeline was extensively modified to support alias generation with configurable delimiters. The SanitizeDirName
function (formerly projectNameFromDir
) now accepts a delimiter parameter for flexible alias formatting.
The changes maintain backward compatibility by making aliases optional and providing fallback mechanisms throughout the system. When no alias is configured, the system falls back to using the original project name, ensuring existing deployments continue to work unchanged.
Confidence score: 2/5
- This PR has a critical logic bug in the Terragrunt alias generation that will cause inconsistent behavior
- Score lowered due to duplicate alias assignment in
createBaseProject
function that overwrites previous values - Pay close attention to
libs/digger_config/terragrunt/tac/generate.go
where alias assignment logic conflicts
18 files reviewed, 3 comments
@@ -184,13 +184,13 @@ func SetPRStatusForJobs(prService ci.PullRequestService, prNumber int, jobs []sc | |||
"prNumber", prNumber, | |||
"project", job.ProjectName, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Consider enriching debug logs with the alias information to improve traceability when troubleshooting status updates
Context Used: Context - Enrich debug logs with relevant context. Always include identifiers like 'prNumber' in logs to facilitate easier tracing and troubleshooting. (link)
project.Alias = SanitizeDirName(projectName, aliasDelimeter) | ||
} | ||
if createWorkspace { | ||
project.Workspace = projectName | ||
} | ||
alias := SanitizeDirName(project.Dir, aliasDelimeter) | ||
project.Alias = alias |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: Alias is being set twice with potentially different values. First it's set from SanitizeDirName(projectName, aliasDelimeter)
on line 335, then unconditionally overwritten with SanitizeDirName(project.Dir, aliasDelimeter)
on line 341. This could cause inconsistent behavior.
project.Alias = SanitizeDirName(projectName, aliasDelimeter) | |
} | |
if createWorkspace { | |
project.Workspace = projectName | |
} | |
alias := SanitizeDirName(project.Dir, aliasDelimeter) | |
project.Alias = alias | |
if createProjectName { | |
project.Name = projectName | |
project.Alias = SanitizeDirName(projectName, aliasDelimeter) | |
} | |
if createWorkspace { | |
project.Workspace = projectName | |
} | |
if !createProjectName { | |
alias := SanitizeDirName(project.Dir, aliasDelimeter) | |
project.Alias = alias | |
} |
@@ -470,19 +456,19 @@ func createProject(ignoreParentTerragrunt bool, ignoreDependencyBlocks bool, git | |||
return project, potentialProjectDependencies, nil | |||
} | |||
|
|||
func projectNameFromDir(projectDir string) string { | |||
func SanitizeDirName(projectDir string, delimeter string) string { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
syntax: Parameter name 'delimeter' is misspelled, should be 'delimiter'
Bug SummaryTotal Bugs Found: 1 Critical Bug
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Greptile Summary
This review covers only the changes made since the last review (commit 7e7e819), not the entire PR.
The latest changes address the issues identified in previous reviews by implementing the proper alias assignment logic and fixing the spelling of 'delimiter' throughout the codebase. The key improvement is in the createBaseProject
function where the alias assignment logic has been restructured to avoid the double assignment problem. Now, when createProjectName
is true, the alias is correctly set from the sanitized project name rather than being unconditionally overwritten with the directory path. This ensures consistent alias generation that respects the project configuration settings.
The changes also include renaming the SanitizeDirName
function parameter from 'delimeter' to 'delimiter', correcting a long-standing typo. These modifications work together to provide proper alias support for project names with configurable delimiters, which integrates with the existing Digger configuration system that manages project generation and validation.
Confidence score: 4/5
- This PR is safe to merge with minimal risk as it addresses previously identified logic issues
- Score reflects the fix of a critical logic bug and spelling correction with straightforward implementation
- No files require special attention as the changes are well-contained and address specific issues
1 file reviewed, no comments
if createProjectName || createWorkspace { | ||
projectName := projectNameFromDir(project.Dir) | ||
projectName := SanitizeDirName(project.Dir, "_") | ||
if createProjectName { | ||
project.Name = projectName | ||
project.Alias = SanitizeDirName(projectName, aliasDelimeter) | ||
} | ||
if createWorkspace { | ||
project.Workspace = projectName | ||
} | ||
alias := SanitizeDirName(project.Dir, aliasDelimeter) | ||
project.Alias = alias | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the createBaseProject
function, there's a bug where the ProjectAlias
field is set twice. First, it's set to the sanitized project name when createProjectName
is true, but then it's immediately overwritten with the sanitized directory name regardless of the createProjectName
condition. This causes the ProjectAlias
to always be based on the directory name, ignoring any customization that might have been intended through the project name.
The second assignment of project.Alias
should be removed as it unconditionally overwrites the previous assignment, making the conditional assignment meaningless.
if createProjectName || createWorkspace { | |
projectName := projectNameFromDir(project.Dir) | |
projectName := SanitizeDirName(project.Dir, "_") | |
if createProjectName { | |
project.Name = projectName | |
project.Alias = SanitizeDirName(projectName, aliasDelimeter) | |
} | |
if createWorkspace { | |
project.Workspace = projectName | |
} | |
alias := SanitizeDirName(project.Dir, aliasDelimeter) | |
project.Alias = alias | |
} | |
if createProjectName || createWorkspace { | |
projectName := SanitizeDirName(project.Dir, "_") | |
if createProjectName { | |
project.Name = projectName | |
project.Alias = SanitizeDirName(projectName, aliasDelimeter) | |
} | |
if createWorkspace { | |
project.Workspace = projectName | |
} | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Greptile Summary
This review covers only the changes made since the last review (commit 225f036), not the entire PR.
The most recent changes focus on adding comprehensive test coverage for the new alias delimiter feature in the Terragrunt configuration parser. Three key updates were made:
-
Test Infrastructure Updates: The
runTest
function ingenerate_test.go
was enhanced to accept analiasDelimiter
parameter, which is now passed to theParse
function. All existing test calls were updated to use the default underscore delimiter "_" to maintain backward compatibility. -
New Test Case: A dedicated test
TestWithProjectNamesAndAliasDelimiter
was added to validate the alias delimiter functionality using a forward slash "/" as the delimiter, ensuring the feature works with alternative delimiters beyond the default underscore. -
Golden Test Files: Two golden files were updated/added to provide expected output validation. The
withProjectName.yaml
file now includes analias
field matching the project name, while the newwithProjectNameAndAliasDelimiter.yaml
demonstrates how the same directory path generates different formatted names and aliases based on their respective delimiters.
These changes integrate with the existing Terragrunt configuration parsing system by extending the Parse
function's capabilities while maintaining full backward compatibility. The alias feature allows projects to have both sanitized names (for internal processing) and human-readable aliases (for display), addressing user experience needs while preserving system functionality.
Confidence score: 5/5
- This PR is extremely safe to merge with minimal risk
- Score reflects comprehensive test coverage and simple test data updates with no logic changes
- No files require special attention as these are purely test infrastructure and validation updates
3 files reviewed, no comments
Summary of Bugs Found in CodebaseA total of 5 bugs were identified in the codebase, primarily related to project name handling and code duplication: Critical Bugs
These inconsistencies create a user experience issue where project names are displayed differently across various parts of the application, rather than consistently using the alias when available. |
func GetProjectAlias(j SerializedJob) string { | ||
if j.ProjectAlias != "" { | ||
return j.ProjectAlias | ||
} | ||
return j.ProjectName | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are duplicate implementations of GetProjectAlias
in libs/scheduler/models.go
and libs/scheduler/jobs.go
with identical logic. This creates a maintenance issue where changes to one implementation might not be reflected in the other, leading to inconsistent behavior.
The function in models.go
operates on SerializedJob
while the one in jobs.go
operates on Job
. Both functions serve the same purpose - to return the project alias if it exists, otherwise return the project name.
I've added a comment to the function in models.go
to clarify its purpose. A more comprehensive fix would be to refactor these functions to avoid duplication, possibly by:
- Creating a common interface that both types implement
- Moving the logic to a utility function that accepts the necessary fields
- Ensuring that any changes to one implementation are reflected in the other
For now, the added documentation helps clarify the purpose and makes it more obvious that these functions are related.
func GetProjectAlias(j SerializedJob) string { | |
if j.ProjectAlias != "" { | |
return j.ProjectAlias | |
} | |
return j.ProjectName | |
} | |
// GetProjectAlias returns the project alias if it exists, otherwise returns the project name | |
// This is a utility function used across the codebase to get a display name for projects | |
func GetProjectAlias(j SerializedJob) string { | |
if j.ProjectAlias != "" { | |
return j.ProjectAlias | |
} | |
return j.ProjectName | |
} |