Feat/stackitsdk 517 service deletion - #290
Conversation
|
|
||
| case "${LANGUAGE}" in | ||
| go) | ||
| rm -rf "services/${service}" "examples/${service}" |
There was a problem hiding this comment.
we use relative paths here with the rm command. Maybe we should establish the working directory in the beginning of the loop? Or use an absolute path for the rm command here? Otherwise cds before the loop might break this
There was a problem hiding this comment.
cd in line 62 before the previous loop starts
| oasService := entry.Name() | ||
| service := language.NormalizeServiceName(oasService) | ||
| if service == "" { | ||
| return Plan{}, fmt.Errorf("OAS service %q has no valid service name", oasService) | ||
| } | ||
| if previous, exists := services[service]; exists { | ||
| return Plan{}, fmt.Errorf("OAS services %q and %q both normalize to %q", previous.OASService, oasService, service) | ||
| } | ||
|
|
||
| action := "generate" | ||
| if _, isBlocked := blocked[service]; isBlocked { | ||
| action = "block" | ||
| } else if len(params.IncludedService) > 0 && !isIncluded(params.IncludedService, service) { | ||
| action = "not_included" | ||
| } | ||
| services[service] = PlannedService{Service: service, OASService: oasService, Action: action} | ||
| } | ||
|
|
||
| serviceEntries, err := fs.ReadDir(filesystem, params.ServiceDir) | ||
| if err != nil { | ||
| return Plan{}, fmt.Errorf("read service directory: %w", err) | ||
| } | ||
| for _, entry := range serviceEntries { | ||
| if !entry.IsDir() { | ||
| continue | ||
| } | ||
| service := entry.Name() | ||
| if _, exists := services[service]; !exists { | ||
| services[service] = PlannedService{Service: service, Action: "delete"} | ||
| } | ||
| } |
There was a problem hiding this comment.
It looks like when a service is on the block list and removed from the spec, it does not get the delete action but block action. Is that intended?
| --spec-dir "oas/services" \ | ||
| --service-dir "sdk-repo-updated/services" \ | ||
| --blocklist "languages/golang/blocklist.txt" \ | ||
| --output "generation-plan.json" |
There was a problem hiding this comment.
Maybe we should name the output langeuage-specific, so there is no possibility of concurrent scripts overriding each others plans
There was a problem hiding this comment.
maybe a feature for later, concurrent execution would also clobber the sdk-repo-updated dir.
| FS: rootFileSystem{root: root}, | ||
| } | ||
|
|
||
| if ok := run(ctx, stdio); !ok { |
There was a problem hiding this comment.
why do all run functions return a bool instead of the error? this makes it harder to debug, no?
There was a problem hiding this comment.
refactored runGenerate etc. to return an error instead. Errors are loggen in main.go/run as the first function that receives an Environment struct wit Err as logging writer.
main.go/run still returns a bool to communicate which exit code to use to main itself.
| type IO struct { | ||
| Args []string | ||
| In io.Reader | ||
| Out io.Writer | ||
| Err io.Writer | ||
| FS FileSystem | ||
| } |
There was a problem hiding this comment.
Nitpick: maybe let's pick another name since we also import the IO package here. Maybe call it Streams?
There was a problem hiding this comment.
renamed to Envrionment
| return Plan{}, fmt.Errorf("OAS services %q and %q both normalize to %q", previous.OASService, oasService, service) | ||
| } | ||
|
|
||
| action := "generate" |
There was a problem hiding this comment.
Nitpick: might be worth introducing a action type here with explicit support actions instead of plain strings
There was a problem hiding this comment.
nice idea, introduced Action type and constants
Co-authored-by: Jonas Schlecht <73650029+SerseusWasTaken@users.noreply.github.com>
Introduce go build tool to generate and read plans.
Use go build tool to create service deletion PRs.
Review Instructions
feat(ci): create PRs for deleted branches, unify plan locationcreates PRs for deleted services and contains new codefeat(ci): sdk-create-pr does not push/create PR when NO_PUSH is setadds a NO_PUSH option to create-pr for easier local testingTesting Instructions
Testing refactorings:
diffoscope --no-default-limits --markdown diff-python.md --max-diff-block-lines 10000 --max-report-size 0 --exclude "**/.git" --exclude-command "stat.*" --exclude "**/.venv" --exclude "**/.gradle" base-python-sdk branch-python-sdk/Testing service deletion:
make download-oasandmake generate-go-sdkNO_PUSH=1 scripts/sdk-create-pr.sh foo barTestscript: