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 Piece and Store namespaces for easier importing types #138

Merged
merged 5 commits into from
Nov 14, 2021
Merged

feat: add Piece and Store namespaces for easier importing types #138

merged 5 commits into from
Nov 14, 2021

Conversation

Lioness100
Copy link
Contributor

Addon to #136. I'm not sure if I should make a new namespace for AliasStore, since it would literally be a duplicate of the Store namespace, or just accept it being inconsistent because AliasPiece has a namespace.

src/lib/structures/Piece.ts Outdated Show resolved Hide resolved
src/lib/structures/Store.ts Outdated Show resolved Hide resolved
src/lib/structures/AliasPiece.ts Outdated Show resolved Hide resolved
@favna
Copy link
Member

favna commented Nov 14, 2021

I'm not sure if I should make a new namespace for AliasStore, since it would literally be a duplicate of the Store namespace, or just accept it being inconsistent because AliasPiece has a namespace.

I would say no. End-users won't really often be using AliasStore for their own code anyway because aliases are really only useful for message based commands. They're completely useless for context menu commands and pretty useless for slash commands.

(p.s. I have no idea what vladdy's plans are for handling aliases for slashies, or if he even gave that any thought yet)

@favna favna changed the title refactor: add Store namespace and add to Piece namespace feat: add Piece and Store namespaces for easier importing types Nov 14, 2021
@favna favna merged commit 84e2d24 into sapphiredev:main Nov 14, 2021
@favna
Copy link
Member

favna commented Nov 14, 2021

Changed the title / commit message because I realized there was no feat of this yet.

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.

None yet

2 participants