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

moose cleanup #2053

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

moose cleanup #2053

wants to merge 2 commits into from

Conversation

DatGuyJonathan
Copy link
Contributor

@DatGuyJonathan DatGuyJonathan commented Feb 21, 2025

  • Remove dcm related cli commands bump-version and generate migrations
  • Remove unused dcm functions
  • Remove leftover core_v1 code

Copy link

vercel bot commented Feb 21, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
framework-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 21, 2025 0:37am
framework-landing ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 21, 2025 0:37am
moose-logs-frontend ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 21, 2025 0:37am
moose-product-analytics ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 21, 2025 0:37am

@DatGuyJonathan DatGuyJonathan marked this pull request as ready for review February 21, 2025 00:52
Copy link
Collaborator

@callicles callicles left a comment

Choose a reason for hiding this comment

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

Looks like some stuff should not be deleted and some more could?

@@ -58,12 +58,8 @@ pub enum Commands {
Dev {},
/// Start a remote environment for use in cloud deployments
Prod {},
/// Generates missing migration files
/// Generates sdk for your data models
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is also used to generate the hash function - so probably not accurate helper

};
Ok(())
}

pub async fn resume_initial_data_load(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't this be deleted as well?

let available_topics: HashSet<String, RandomState> =
HashSet::from_iter(fetch_topics(&kafka_config).await?);

// Spawn sync for the current and old models
Copy link
Collaborator

Choose a reason for hiding this comment

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

That feels like it should not get deleted

@callicles
Copy link
Collaborator

Also a callout for something that needs to be tested - with DCM we were using version with naming DMs with removing DCM we probably should remove the naming versioning?

@callicles
Copy link
Collaborator

or we could keep it - not sure

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