feat: Include updated images in commit message

[tsmalls93]

Fixes #48

[bluebrown]

Hi, thank you for the effort! I am a little bit confused about this design. Like moving it into the decoder.

What is the benefit of this vs ranging over the actual changes reported by the krm filter?

[bluebrown]

Dont worry about the ci failure. I put a failing test in main, on purpose. I will fix that one now, then you could rebase main to get this CI green.

[bluebrown]

I have fixed the test a92a2d2

[tsmalls93]

Hi, thank you for the effort! I am a little bit confused about this design. Like moving it into the decoder.

What is the benefit of this vs ranging over the actual changes reported by the krm filter?

So, the solution you had in mind was to construct the commit message from the changes array that is returned from the Pipeline function? That probably would have been easier if I had thought of that. I suppose the only benefit to my solution is it gives more control to the user, but either solution would work for my use case.

[tsmalls93]

Oh, I see. The image(s) being sent to the decoder won't necessarily result in a change.

[bluebrown]

Oh, I see. The image(s) being sent to the decoder won't necessarily result in a change.

Yes, that is one thing. And another thing is that multiple events can could run as batch. So it may change serveral image refs, in your repo, at once. Depending on the debounce timer and so on.

I did experiment with tracking changes in the past, doing file diffs and so on. But at the end a semantic diff is probably the best, in order to avoid false positives due to things like white space and indentation changes.

So I found it most solid to have the filter track and report the changes it makes.

[bluebrown]

I think we could make changes a slice of structs with structured data so we don't have to use regex.

[bluebrown]

I guess here the type needs to be changed to some struct type:

kobold/krm/filter.go

Line 53 in a92a2d2

Changes []string

[bluebrown]

you can run the checks locally

kobold/Makefile

Line 40 in a92a2d2

###@ Checks

. golangci has also something like a --fix flag, Iu think.

[tsmalls93]

you can run the checks locally

kobold/Makefile

Line 40 in a92a2d2

###@ Checks

. golangci has also something like a --fix flag, Iu think.

Yeah, I'm getting different errors and the linting seems to give up after them. It is not liking the copyDir file seemingly because I am on an arm64 Mac and not x86_64 Linux. I think I got the import issues fixed though.

[tsmalls93]

Maybe I can make an Apple Silicon specific Makefile in a future PR.

[bluebrown]

you can run the checks locally

kobold/Makefile

Line 40 in a92a2d2

###@ Checks

. golangci has also something like a --fix flag, Iu think.

Yeah, I'm getting different errors and the linting seems to give up after them. It is not liking the copyDir file seemingly because I am on an arm64 Mac and not x86_64 Linux. I think I got the import issues fixed though.

Go has build tags. Some of them are architecture specific. As a shortcut, if you suffix a file with an architecutre, it will be only build on that arch, like if you would have used a build tag.

You could create a file called copy_darwin.go and put some mac specific copy command there. The reason its guarded is because its using exec to run an external command.

Like this one, but for darwin: https://github.com/bluebrown/kobold/blob/main/git/copy_linux.go

[tsmalls93]

Gotcha. I ended up renaming it to copy.go and adding //go:build linux || darwin to the top of the file, since it would be the same code. That made the error go away. I didn't commit it since my build issues are out of scope for this PR, but I'll do some work to get it to be compatible with arm64 Darwin.

[bluebrown]

Hi, thanks. Do you think its good now ? Should we maybe add a few tests?

[tsmalls93]

Yeah, I think it is good now. I just added a couple tests.

[bluebrown]

hi @tsmalls93 , thanks. I build and pushed the latest main as image: docker.io/bluebrown/kobold:aab019c

Its not released yet but you can use that one already, if you want.