Skip to content
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

refactor: improve imagerunner cloudevent handling #879

Merged
merged 39 commits into from
Jan 29, 2024
Merged

Conversation

alexplischke
Copy link
Contributor

@alexplischke alexplischke commented Jan 26, 2024

Proposed changes

Try to improve code quality and clarity of the cloudevent handling logic (e.g. live-logs).

No business logic changes per se, but some methods were more drastically altered than others (some inversion of booleans, consolidation or breaking apart methods) which results in internal logical changes of the program.

Out of Scope:
imagerunner/async.go contains code to handle cloudevents and websockets. As this is code that's more related to HTTP than ImageRunner itself, I'd love to move this to the http package, but not in this PR.

PS: I recommend going through the individual commits, so as to better understand the changes (I know, it's plenty!).

@alexplischke alexplischke added the ignore-for-release Pull requests that aren't release relevant label Jan 26, 2024
@alexplischke alexplischke marked this pull request as ready for review January 26, 2024 22:58
@alexplischke alexplischke requested a review from a team as a code owner January 26, 2024 22:58
internal/http/imagerunner.go Outdated Show resolved Hide resolved
internal/http/imagerunner.go Outdated Show resolved Hide resolved
internal/http/imagerunner.go Outdated Show resolved Hide resolved
internal/imagerunner/errors.go Show resolved Hide resolved
internal/saucecloud/imagerunner.go Outdated Show resolved Hide resolved
@alexplischke alexplischke merged commit fd15b95 into main Jan 29, 2024
18 checks passed
@alexplischke alexplischke deleted the DEVX-2713 branch January 29, 2024 18:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ignore-for-release Pull requests that aren't release relevant
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants