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

Remove logging "UI" utils from this library #108

Merged
merged 4 commits into from
Mar 29, 2024

Conversation

brandenrodgers
Copy link
Contributor

Description and Context

We had a discussion about adding these logging utils a while ago here.

Now that we're starting to wire up the exports from this library into the CLI, I'm changing my mind about this pattern of including logging utils in here. The goal for this library is to enable us to separate the core business logic from the "UI" (aka CLI). Keeping UI related components in here will put us back into a spot where the dividing line between what goes in the CLI and what goes in this library will be blurry. One of the initial tenants for this lib was to make it as environment agnostic as possible, and keeping ui components in here doesn't really fit into that ideal.

I have a corresponding PR in the CLI where I port all of the deleted files from here back into the CLI. There's a tiny bit of duplicate code in some of the other packages, but IMO that's acceptable. I think eventually we should try to split out the webpack-cms-plugins and serverless-dev-runtime packages out into their own repos. They're a remnant from back when we treated the CLI repo as a monorepo, but we're moving away from that pattern. Duplicating the code in those packages will seem less gross if/when we move them out into their own repos.

Screenshots

TODO

Who to Notify

Copy link
Contributor

@camden11 camden11 left a comment

Choose a reason for hiding this comment

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

Definitely need to make sure this is properly tested, but the code all looks good to me

@brandenrodgers brandenrodgers merged commit 230324c into main Mar 29, 2024
1 check passed
@brandenrodgers brandenrodgers deleted the br-remove-logging-utils branch March 29, 2024 15:57
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.

2 participants