-
-
Notifications
You must be signed in to change notification settings - Fork 68
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
refactor: rewrite to djs v14 #2505
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #2505 +/- ##
==========================================
- Coverage 90.27% 88.73% -1.54%
==========================================
Files 88 88
Lines 3618 3630 +12
Branches 298 281 -17
==========================================
- Hits 3266 3221 -45
- Misses 349 403 +54
- Partials 3 6 +3 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I may have made some mistakes in reviewing because there were so many files
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now with request changes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple of comments
tests/lib/database/settings/structures/PermissionNodeManager.test.ts
Outdated
Show resolved
Hide resolved
tests/lib/database/settings/structures/PermissionNodeManager.test.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Skyra is now officially on Sapphire v5! skyra-project/skyra#2505
Skyra is now officially on Sapphire v5! skyra-project/skyra#2505
👀
Changes
Downgraded Node.js CI version from 20 to 18 (and pinned to v18 with Volta) because import loader issues in Workers with Node.js v20. See: New--import
API breaks worker threads privatenumber/tsx#354Because the loader in v18 can resolve.ts
inWorker
's constructor, and is required to be passed in the arguments (rather than done at runtime),scripts/workerTsLoader.mjs
was deleted.jest
tovitest
for better test performance.Switched fromRemovedts-node
totsx
for better support in Node.js v18 and upwards (usesesbuild
under the hood as well).ts-node
altogether.discord.js
from v13 to v14.discord-api-types/v9
todiscord-api-types/v10
to match.@sapphire/framework
from v2 to v5.typeorm
, giving them a last chance. Skyra now uses less relationships, so it should be probably fine.SkyraCommand
(extendsSubcommand
) intoSkyraCommand
(extendsCommand
) andSkyraSubcommand
(extendsSubcommand
).PaginatedMessageCommand
, replaced withSkyra{Subc,C}ommand.PaginatedOptions
to reduce code duplication.v7-*
commands. Keptv7-artiel
since it's newer and expected to be used soon.banners
andclient
)./banners
).TriviaManager
tests, they're being removed soon in favour of Artiel.Extras
s!conf
's menu to listen only to reactions on the menu's messages!dice
to default parameter-less to6
getImage
to fallback to stickers when there are no attachmentschannelName
argument to throw a less misleading error when filter returns false