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

feat: add install/uninstall validation #1221

Merged
merged 5 commits into from
Dec 20, 2024

Conversation

howydev
Copy link
Collaborator

@howydev howydev commented Dec 14, 2024

TODO: fix inconsistency between entityId types - we use bigints somewhere and numbers in other places


PR-Codex overview

This PR introduces the EntityIdOverrideError class to handle cases where an entity ID is invalid due to overriding the owner's entity ID. Additionally, it adds new types and utilities for validation configurations and modifies several functions to integrate these changes.

Detailed summary

  • Added EntityIdOverrideError class in client.ts to signify invalid entity ID overrides.
  • Introduced DEFAULT_OWNER_ENTITY_ID constant.
  • Expanded ValidationConfig, HookConfig, and ModuleEntity types in types.ts.
  • Updated client functions to use SMAV2AccountClient.
  • Enhanced validation actions with new parameters and checks.
  • Modified various test cases to include new validation features.

✨ Ask PR-Codex anything about this PR by commenting with /codex {your question}

Copy link

vercel bot commented Dec 14, 2024

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

Name Status Preview Comments Updated (UTC)
aa-sdk-site ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 20, 2024 10:18pm
aa-sdk-ui-demo ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 20, 2024 10:18pm

Copy link

graphite-app bot commented Dec 14, 2024

How to use the Graphite Merge Queue

Add the label graphite-merge-queue to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

);
}

if (validationConfig.entityId === 0) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Mental note for ourselves: This is only the case for SMAb

@howydev howydev force-pushed the howy/add-install-validation branch from 9e5c594 to 65965de Compare December 17, 2024 18:15
@howydev howydev changed the title [draft] feat: add install/uninstall validation feat: add install/uninstall validation Dec 17, 2024
@howydev howydev force-pushed the howy/add-install-validation branch from 74e34e6 to 759c516 Compare December 17, 2024 21:50
@howydev howydev force-pushed the howy/scaffold-v0.8 branch 3 times, most recently from f46ac29 to e09b3f3 Compare December 17, 2024 23:45
@howydev howydev force-pushed the howy/add-install-validation branch from 759c516 to 8f31eb6 Compare December 17, 2024 23:49
@howydev howydev force-pushed the howy/add-install-validation branch from 8f31eb6 to a82e838 Compare December 18, 2024 01:46
@howydev howydev force-pushed the howy/add-install-validation branch 3 times, most recently from cbf1d53 to be95010 Compare December 18, 2024 07:47
@howydev howydev force-pushed the howy/add-install-validation branch from d161f12 to 4e59a6f Compare December 20, 2024 22:02
@howydev howydev merged commit 4677003 into howy/scaffold-v0.8 Dec 20, 2024
3 of 6 checks passed
@howydev howydev deleted the howy/add-install-validation branch December 20, 2024 22:02

import { createSMAV2AccountClient } from "./client.js";

import { createSMAV2AccountClient, type SMAV2AccountClient } from "./client.js";
Copy link
Collaborator

Choose a reason for hiding this comment

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

⚠️ [eslint] <@typescript-eslint/no-unused-vars> reported by reviewdog 🐶
'SMAV2AccountClient' is defined but never used.

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.

4 participants