bashbot icon indicating copy to clipboard operation
bashbot copied to clipboard

refactor: bashbot slack logic refactor

Open wisdommatt opened this issue 2 years ago • 3 comments

@mathew-fleisch I am currently refactoring the bashbot slack logic, I have changed function names and added comment/documentation to some of the codes. the new logic is in internal/bashbot. Am still in the process of refactoring the logic. Have been very busy lately, so just look at what i have done so far and drop your feedback/review.

wisdommatt avatar Apr 12 '22 23:04 wisdommatt

It is looking good so far! I like your changes. When you achieve parity, we can delete the old .go file. I've set up some automation in this branch that isn't working because you need to point at the new location (internal/slack/slack.go) here: https://github.com/wisdommatt/bashbot/blob/cobra-cli/Makefile#L6

Once you do, it will build and test the changes in a KinD cluster within github actions. I hope you can see the console-log/output from the automation by clicking the 'Details' links next to the failed tests PR Tests / Lint and Unit Tests (pull_request) and PR Tests / KinD Integration Tests (pull_request)

Maybe we can split out the struct definitions to a separate file?

mathew-fleisch avatar Apr 15 '22 22:04 mathew-fleisch

It is looking good so far! I like your changes. When you achieve parity, we can delete the old .go file. I've set up some automation in this branch that isn't working because you need to point at the new location (internal/slack/slack.go) here: https://github.com/wisdommatt/bashbot/blob/cobra-cli/Makefile#L6

Once you do, it will build and test the changes in a KinD cluster within github actions. I hope you can see the console-log/output from the automation by clicking the 'Details' links next to the failed tests PR Tests / Lint and Unit Tests (pull_request) and PR Tests / KinD Integration Tests (pull_request)

Maybe we can split out the struct definitions to a separate file?

Splitting the struct definitions to a separate file will be a good idea

wisdommatt avatar Apr 20 '22 08:04 wisdommatt

Hey @wisdommatt can you rebase your branch? I'd like to finally get this merged, but it is a bit out of date now.

mathew-fleisch avatar Sep 18 '22 14:09 mathew-fleisch