-
-
Notifications
You must be signed in to change notification settings - Fork 100
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
chore: Code changes & formatting and linting #3552
Open
Fleny113
wants to merge
5
commits into
main
Choose a base branch
from
refactor-&-formatting
base: main
Could not load branches
Branch not found: {{ refName }}
Could not load tags
Nothing to show
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
+431
−562
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
w-unverified
This has not been verified
pkg-all
Affects every package
pkg-bot
Affects the bot package
pkg-gateway
Affects the gateway package
pkg-rest
Affects the rest package
pkg-types
Affects the types package
pkg-utils
Affects the utils package
t-ci
Changes to our CI configuration files and scripts
website
Affects the website
labels
May 6, 2024
Fleny113
added
t-refactor
A code change that neither fixes a bug nor adds a feature
t-style
Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, et
t-build
Changes that affect the build system or external dependencies
github_actions
Pull requests that update GitHub Actions code
labels
May 6, 2024
lts20050703
reviewed
May 7, 2024
lts20050703
approved these changes
May 7, 2024
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!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
github_actions
Pull requests that update GitHub Actions code
pkg-all
Affects every package
pkg-bot
Affects the bot package
pkg-gateway
Affects the gateway package
pkg-rest
Affects the rest package
pkg-types
Affects the types package
pkg-utils
Affects the utils package
t-build
Changes that affect the build system or external dependencies
t-ci
Changes to our CI configuration files and scripts
t-refactor
A code change that neither fixes a bug nor adds a feature
t-style
Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, et
w-unverified
This has not been verified
website
Affects the website
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I will explain pretty much everything in the PR, but a lot of the changes are the result of running prettier on the entire project and ESLint --fix on most of the files.
The formatting is taking most of the file changes with very minimal ones often but might as well run it one on everything than everytime we need to touch that file (even if by a merge) eslint/prettier edits them and then we got to reverse that change because it's unrelated.
.devcontainer/devcontainer.json
: Add ayarn install
after the dev container is created. Currently if you create a GitHub CodeSpace for example the VSCode task for the build:watch fails as it is missing the node_modules folder.packages/benchmark
: This does affect some other files, like.github/workflows/benchmark.yml
, the reason this got changed is because while I was testing the benchmarks node (v22 at least) was failing to resolve thebenchmark
package (the npm package), because it was actually trying to resolve the package itself, so I renamed the package tobenchmarks
in thepackage.json
, but at that point I have just renamed the folder..npmignore
: On the npm package we have the.tsbuildinfo
and we don't need it in a package, so we can just remove it.README.md
: Add the bun unit test script, since there was already the deno unit script, might as well add the bun unit script.packages/benchmarks/package.json
: Themain
&exports
values were expecting the dist folder to also have the cjs files, but the scripts are to build only with esm, so to make it run i have fixed the values.packages/benchmarks/src/benchmarks/casting.ts
: TheCamelize 1 event
was actually running snakelize on an already snakelized event, so it didn't make any sense.packages/benchmarks/src/benchmarks/memory.ts
: The old code was achiving the same thing assnakeToCamelCase
, so no reason to use that code instead of the function we already have.packages/bot/tests/e2e/resetguilds.spec.ts
tsconfig.json
:tsconfig.test.json
: was not getting picked up by typescript (at least in vscode, tested also with a clean codespace) so it got moved to the test directory with thetsconfig.json
name and it seems to be getting picked up by typescript.rootDir
for the source, use an include pattern instead of an exclude one and set explicitly what is the path for the.tsbuildinfo
file.packages/tsconfig/base.json
: The exclude pattern is actually relative to where the base.json is, so it wasn't doing anything. It will be the case untile TS 5.5 where we will be able to use${configDir}
to set things relative to the destinationtsconfig.json
, so mosttsconfig.json
will provably become just a json with an export like the tests ones. TS 5.5 Beta, ${configDir}. TheskipDefaultLib
is just skipping the check for typescript.d.ts
files.packages/gateway/tests/integration/connection.spec.ts
: ESLint warnings/errors mainlyscripts/checkCpuModel.js
: ESLint errorpackages/rest/tests/e2e/utils.ts
: This affected most if not all e2e files, just a rename to use camelCase instead of all lowercase.scripts/finalizeTypedocs.js
: Node 22 compatibility, node 22 removed support forassert
in imports statements and the import function, so it broke when run with node 22. This does not affect node 18+ as it seems to be working under a built of the latest LTS of node 18.package.json
:packages/benchmarks/package.json
:microtime
is not used,node-fetch
is not necessary considering we support only node 18+ where the built-in fetch got stable.packages/rest/package.json
: We do usedotenv
for our tests (to use the.env
file in the root of the directory in e2e tests) but we don't need to specifiy it in the "prod" dependencies.why-is-node-running
is not used, and doesn't seem to have a reason to be there eitherpackages/types/package.json
: In the types package we just have an "import correctly" test, so chai is not necessary, also@types/simon
withoutsimon
installed is not doing anything, just creating clutter in thepackage.json
.packages/utils/package.json
: While we might want to readd the public key validation for HTTP interactions (from what I understand, at some point we had the code to do support this), currently we don't needtweetnacl
to be installed.If there is something that isn't clear why it got edited, i will do my best to respond with a reason, reason that might just prettier/eslint that edited or why i did edit it.