From cd03fcd8b5779239a51b289a9a7c79435469061c Mon Sep 17 00:00:00 2001 From: Martin Najemi Date: Sun, 7 Jun 2026 03:51:03 +0200 Subject: [PATCH] chore: Remove app-relationship target feature Risk: low --- CHANGELOG.md | 6 +++ README.md | 8 +--- VERSION | 2 +- internal/analyzer/analyzer.go | 82 ++--------------------------------- internal/rush/rush.go | 1 - main.go | 30 ------------- 6 files changed, 11 insertions(+), 118 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 33d3422..997e349 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,11 @@ 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.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [0.23.0] - 2026-06-07 + +### Removed +- **Breaking:** The `app` field on target definitions in `.goodchangesrc.json` (the "app-relationship" feature) and its `IGNORE_APP_RELATIONSHIP` env var are removed. Targets are no longer triggered just because a referenced app is affected — linking e2e targets to applications is the caller's responsibility, done after change detection. While this was helpful to us initially, it was later decided to be outside the scope of the change detector, which should be repository agnostic. It also conflated two distinct questions ("which apps need testing" vs "which e2e libs changed directly"), and forced a second invocation with `IGNORE_APP_RELATIONSHIP` to surface fine-grained detections the app check would otherwise hide. The `app` field is now silently ignored if present. Also removed the now-unused `analyzer.HasTaintedImports` helper. + ## [0.22.0] - 2026-06-07 ### Changed @@ -312,6 +317,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Multi-stage Docker build - Automated vendor upgrade workflow +[0.23.0]: https://github.com/gooddata/gooddata-goodchanges/compare/v0.22.0...v0.23.0 [0.22.0]: https://github.com/gooddata/gooddata-goodchanges/compare/v0.21.3...v0.22.0 [0.21.3]: https://github.com/gooddata/gooddata-goodchanges/compare/v0.21.2...v0.21.3 [0.21.2]: https://github.com/gooddata/gooddata-goodchanges/compare/v0.21.1...v0.21.2 diff --git a/README.md b/README.md index 8ed32b6..ce308f8 100644 --- a/README.md +++ b/README.md @@ -38,7 +38,7 @@ goodchanges --version # print version 5. Computes the full affected subgraph (transitive dependents) 6. Topologically sorts affected packages (dependencies first) 7. For each **library**: parses old and new TypeScript ASTs, diffs symbols, and propagates taint through import graphs -8. For each **target**: checks if it's affected via direct changes, lockfile changes, tainted imports, or a tainted corresponding app +8. For each **target**: checks if it's affected via direct changes, lockfile changes, or tainted imports 9. Outputs a JSON array of affected e2e package names to stdout ## Output @@ -65,7 +65,6 @@ JSON array of target objects: | `COMPARE_COMMIT` | Specific git commit hash to compare against (overrides branch-based comparison) | _(empty)_ | | `COMPARE_BRANCH` | Git branch to compute merge base against | `origin/master` | | `TARGETS` | Comma-delimited list of target names to include in output. Supports `*` wildcard (e.g. `*backstop*,@gooddata/sdk-*`). | _(all targets)_ | -| `IGNORE_APP_RELATIONSHIP` | When set to any non-empty value, ignores the `app` field in target configs. Targets are no longer triggered solely because their corresponding app is tainted. | _(disabled)_ | ## Library vs app detection @@ -88,9 +87,6 @@ Each project can optionally have a `.goodchangesrc.json` file in its root direct ```json { "targets": [ - { - "app": "@gooddata/gdc-dashboards" - }, { "targetName": "neobackstop", "changeDirs": [ @@ -127,7 +123,6 @@ Each target is triggered by any of these conditions: 1. **Direct file changes** -- files matching `changeDirs` globs changed (excluding ignored paths). Defaults to `**/*` (entire project) when `changeDirs` is not set. 2. **External dependency changes** -- a dependency version changed in `pnpm-lock.yaml` 3. **Tainted workspace imports** -- a file matching `changeDirs` globs imports a tainted symbol from a workspace library -4. **Corresponding app is tainted** -- the app specified by `app` is affected (any of the above conditions). Disabled when `IGNORE_APP_RELATIONSHIP` env var is set. ### changeDirs @@ -165,7 +160,6 @@ Each `changeDirs` entry is an object with: | Field | Type | Description | |--------------|---------------|---------------------------------------------------------------------------------------------------------------------------------------------| -| `app` | `string` | Package name of the corresponding app this target tests | | `targetName` | `string` | Custom output name (defaults to the package name when not set) | | `changeDirs` | `ChangeDir[]` | Glob patterns to match files. Defaults to `**/*` (entire project). Each entry: `{"glob": "...", "filter?": "...", "type?": "fine-grained"}` | | `ignores` | `string[]` | Per-target ignore globs. Additive with the global `ignores` -- only applies to this target's detection | diff --git a/VERSION b/VERSION index a7f3fc2..2ba6141 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.22.0 \ No newline at end of file +0.23.0 \ No newline at end of file diff --git a/internal/analyzer/analyzer.go b/internal/analyzer/analyzer.go index 9a9c9b1..63b54fb 100644 --- a/internal/analyzer/analyzer.go +++ b/internal/analyzer/analyzer.go @@ -140,85 +140,9 @@ func collectExportsFromFile(projectFolder, relFile string, seen, visited map[str } } -// HasTaintedImports checks if any source file in the given folder imports -// tainted symbols from the upstreamTaint map. Used for app-like packages -// (e.g. e2e scenario apps) where we don't need to trace to entrypoint exports, -// just detect whether any tainted dependency is actually imported. -func HasTaintedImports(folder string, upstreamTaint map[string]map[string]bool, ignoreCfg *rush.ProjectConfig) bool { - log.Debugf("HasTaintedImports: %s (upstream taint keys: %d)", folder, len(upstreamTaint)) - if len(upstreamTaint) == 0 { - return false - } - allFiles, err := globSourceFiles(folder) - if err != nil { - return false - } - for _, relPath := range allFiles { - if ignoreCfg.IsIgnored(relPath) { - continue - } - fullPath := filepath.Join(folder, relPath) - analysis, err := tsparse.ParseFile(fullPath) - if err != nil { - continue - } - for _, imp := range analysis.Imports { - if strings.HasPrefix(imp.Source, ".") { - continue - } - // Check exact match first (normal TS exports) - affectedNames, ok := upstreamTaint[imp.Source] - if !ok || len(affectedNames) == 0 { - // Check prefix match for CSS taint (e.g. import "@gooddata/pkg/styles/css/main.css" - // matches taint key "@gooddata/pkg/styles") - if IncludeCSS && matchesCSSTaint(imp.Source, upstreamTaint) { - log.Debugf(" HasTaintedImports: %s matched CSS taint via %s", folder, imp.Source) - return true - } - continue - } - if len(imp.Names) == 0 { - // Unassigned import from tainted package - log.Debugf(" HasTaintedImports: %s matched via unassigned import of %s in %s", folder, imp.Source, relPath) - return true - } - for _, name := range imp.Names { - if strings.HasPrefix(name, "*:") { - // Namespace import — any taint means affected - log.Debugf(" HasTaintedImports: %s matched via namespace import of %s in %s", folder, imp.Source, relPath) - return true - } - if affectedNames[name] { - log.Debugf(" HasTaintedImports: %s matched via %s importing %s from %s", folder, relPath, name, imp.Source) - return true - } - } - } - } - - // Also check SCSS files for @use of tainted style packages - if IncludeCSS { - scssFiles := globStyleFiles(folder) - for _, scssFile := range scssFiles { - if ignoreCfg.IsIgnored(scssFile) { - continue - } - uses := parseScssUses(filepath.Join(folder, scssFile)) - for _, useSpec := range uses { - if matchesCSSTaint(useSpec, upstreamTaint) { - log.Debugf(" HasTaintedImports: %s matched CSS taint via SCSS @use %s", folder, useSpec) - return true - } - } - } - } - - return false -} - -// HasTaintedImportsForGlob is like HasTaintedImports but scopes to files matching -// a glob pattern (relative to projectFolder) instead of a flat directory. -// Ignores override glob matches. +// HasTaintedImportsForGlob checks whether any source file matching a glob +// pattern (relative to projectFolder) imports tainted symbols from the +// upstreamTaint map. Ignores override glob matches. func HasTaintedImportsForGlob(projectFolder, globPattern string, upstreamTaint map[string]map[string]bool, ignoreCfg *rush.ProjectConfig) bool { log.Debugf("HasTaintedImportsForGlob: %s (glob=%s, upstream taint keys: %d)", projectFolder, globPattern, len(upstreamTaint)) if len(upstreamTaint) == 0 { diff --git a/internal/rush/rush.go b/internal/rush/rush.go index a6e2bca..d609525 100644 --- a/internal/rush/rush.go +++ b/internal/rush/rush.go @@ -119,7 +119,6 @@ func (cd ChangeDir) IsFineGrained() bool { } type TargetDef struct { - App *string `json:"app,omitempty"` // rush project name of corresponding app TargetName *string `json:"targetName,omitempty"` // custom output name (defaults to package name) ChangeDirs []ChangeDir `json:"changeDirs,omitempty"` // globs to watch (defaults to **/* if empty) Ignores []string `json:"ignores,omitempty"` // per-target ignore globs (additive with global) diff --git a/main.go b/main.go index 71b0ffb..dea0962 100644 --- a/main.go +++ b/main.go @@ -25,7 +25,6 @@ var version string var flagIncludeTypes bool var flagIncludeCSS bool -var flagIgnoreAppRelationship bool var flagLog bool var flagDebug bool @@ -64,7 +63,6 @@ func main() { flagIncludeTypes = envBool("INCLUDE_TYPES") flagIncludeCSS = envBool("INCLUDE_CSS") - flagIgnoreAppRelationship = envBool("IGNORE_APP_RELATIONSHIP") logLevel := strings.ToUpper(os.Getenv("LOG_LEVEL")) flagLog = logLevel == "BASIC" || logLevel == "DEBUG" @@ -136,15 +134,6 @@ func main() { continue } targetSeeds = append(targetSeeds, rp.PackageName) - // Targets that reference an `app` are triggered when that app is - // affected (see the app-taint check below). The app is not an npm - // dependency of the target's own package, so seed it explicitly — - // otherwise the app and its dependency subtree (e.g. the runtime - // library whose change is meant to trigger the e2e target) would be - // excluded from analysis and never produce taint. - if td.App != nil && !flagIgnoreAppRelationship { - targetSeeds = append(targetSeeds, *td.App) - } } } relevantPackages = rush.FindTransitiveDependencies(projectMap, targetSeeds) @@ -434,25 +423,6 @@ func main() { continue } - // Quick check: app taint - if td.App != nil && !flagIgnoreAppRelationship { - appInfo := projectMap[*td.App] - if appInfo != nil { - if changedProjects[*td.App] != nil { - changedE2E[name] = &TargetResult{Name: name} - continue - } - if len(depChangedDeps[appInfo.ProjectFolder]) > 0 { - changedE2E[name] = &TargetResult{Name: name} - continue - } - if analyzer.HasTaintedImports(appInfo.ProjectFolder, allUpstreamTaint, nil) { - changedE2E[name] = &TargetResult{Name: name} - continue - } - } - } - // ChangeDirs detection (defaults to **/* if not configured) changeDirs := td.ChangeDirs if len(changeDirs) == 0 {