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

chore: cleanup all test files #293

Merged
merged 4 commits into from
Sep 27, 2024
Merged

chore: cleanup all test files #293

merged 4 commits into from
Sep 27, 2024

Conversation

Parkreiner
Copy link
Contributor

No issue to link – this is just something I thought I could fix really quickly while working in the codebase.

Changes made

  • Removed all unused imports, and made sure type imports were labeled correctly
  • Updated all comparisons to be more strict
  • Simplified loops to remove unneeded closure functions
  • Removed all explicit any types
  • Updated how strings were defined to follow general TypeScript best practices

Notes

  • We definitely want some kind of linting setup for this repo. I'm going to bring this up when Blueberry has its next team meeting next week

@Parkreiner Parkreiner requested a review from matifali September 20, 2024 18:23
@Parkreiner Parkreiner self-assigned this Sep 20, 2024
Copy link
Member

@matifali matifali left a comment

Choose a reason for hiding this comment

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

I left a comment to prevent any possible merge conflicts for #287. Otherwise all look good to me.

jfrog-oauth/main.test.ts Show resolved Hide resolved
jfrog-token/main.test.ts Show resolved Hide resolved
@matifali
Copy link
Member

@Parkreiner, all modules are merged. You may continue with the refactoring here,

"target": "esnext",
"module": "nodenext",
// If we were just compiling for the tests, we could safely target ESNext at
// all times, but just because we've been starting to add more runtime logic
Copy link
Member

Choose a reason for hiding this comment

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

We only have the runtime logic in the windows-rdp module, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right now, yeah

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But one of the other benefits of having an older compilation target is that, because I have that JavaScript file set up to be part of the TypeScript project, the TypeScript LSP can catch any function calls that might be using features that are too recent

@Parkreiner Parkreiner requested a review from matifali September 27, 2024 19:23
@Parkreiner Parkreiner merged commit 438c904 into main Sep 27, 2024
3 checks passed
@Parkreiner Parkreiner deleted the mes/quick-cleanup branch September 27, 2024 19:35
@matifali
Copy link
Member

We definitely want some kind of linting setup for this repo. I'm going to bring this up when Blueberry has its next team meeting next week

For linting, let us use biomejs, which is similar to a coder/coder. I am using biome vscode extension that does help catch these errors.

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