Skip to content

Conversation

@cjonesy
Copy link
Contributor

@cjonesy cjonesy commented Sep 6, 2024

INF-5526

I think this is the last of the cleanup PRs. Just wanted to move the executioner and watcher implementations into their own packages. This will make it more organized as I add more in subsequent PRs.

@cjonesy cjonesy requested a review from joshuawscott September 6, 2024 21:42
@@ -1,4 +1,4 @@
package executioner
package log
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should name this logger or logging so that it doesn't have the same name as a built in basic package (that we are actually including part of)

@@ -1,4 +1,4 @@
package watcher
package time
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, now you're just trolling.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah... I put those names in as placeholders and planned to swap them out, but forgot. Thanks for the reminder. I'll rename them 🤣

Copy link
Member

@joshuawscott joshuawscott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only comments I have are the confusing/conflicting package names, otherwise LGTM

@cjonesy cjonesy merged commit 3ce13e4 into main Sep 10, 2024
@cjonesy cjonesy deleted the move-to-packages branch September 10, 2024 14:41
@cjonesy cjonesy mentioned this pull request Sep 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants