Skip to content

Require project environment in SDK #14679

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.

Sign up for GitHub

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

Merged
merged 22 commits into from
Nov 20, 2024
Merged

Require project environment in SDK #14679

Changes from 2 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
186822e
Require project environment in SDK
bzwrk Nov 18, 2024
d34e259
Merge branch 'master' into biz/dj-2678-make-environment-mandatory-in-…
bzwrk Nov 18, 2024
647e18c
Refactor environment check into its own method
bzwrk Nov 18, 2024
d91741b
Define a new type for project environment
bzwrk Nov 19, 2024
c1781ed
Fix comment and update CHANGELOG.md
bzwrk Nov 19, 2024
40bab31
Fix linter errors
bzwrk Nov 19, 2024
7753a5b
Fix linter errors
bzwrk Nov 19, 2024
a904e53
Update package lock
bzwrk Nov 19, 2024
5efcc32
Update gitignore
bzwrk Nov 19, 2024
45b4875
Add eslint config
bzwrk Nov 19, 2024
7abedce
Apply eslint
bzwrk Nov 19, 2024
8de3f44
Apply linter suggestions
bzwrk Nov 19, 2024
16704ee
Merge branch 'master' into biz/dj-2678-make-environment-mandatory-in-…
bzwrk Nov 19, 2024
9d5e709
Fix markdown syntax errors
bzwrk Nov 19, 2024
d0362d0
Fix tests
bzwrk Nov 19, 2024
2070190
Merge branch 'master' into biz/dj-2678-make-environment-mandatory-in-…
bzwrk Nov 19, 2024
87fb597
Update pnpm lock
bzwrk Nov 19, 2024
5b86f27
Remove pnpm lock changes
bzwrk Nov 19, 2024
041a594
Merge branch 'master' into biz/dj-2678-make-environment-mandatory-in-…
bzwrk Nov 20, 2024
a171866
Remove eslint configs
bzwrk Nov 20, 2024
96fc56b
Undo eslint change
bzwrk Nov 20, 2024
044daed
Revert reversion of eslint changes
bzwrk Nov 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion packages/sdk/src/server/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,10 @@
* @param opts - The options for configuring the server client.
*/
constructor(opts: BackendClientOpts) {
this.environment = opts.environment ?? "production";
if (!opts.environment || !["development", "production"].includes(opts.environment)) {

Check failure on line 356 in packages/sdk/src/server/index.ts

View workflow job for this annotation

GitHub Actions / Lint Code Base 

A linebreak is required after '['

Check failure on line 356 in packages/sdk/src/server/index.ts

View workflow job for this annotation

GitHub Actions / Lint Code Base 

There should be a linebreak after this element

Check failure on line 356 in packages/sdk/src/server/index.ts

View workflow job for this annotation

GitHub Actions / Lint Code Base 

A linebreak is required before ']'
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Fix array formatting to adhere to style guidelines

The static analysis tools have identified that the array declaration at line 356 requires line breaks after [, after each element, and before ]. This improves readability and complies with the project's style rules.

Apply this diff to fix the formatting:

-        if (!opts.environment || !["development", "production"].includes(opts.environment)) {
+        if (
+          !opts.environment ||
+          ![
+            "development",
+            "production",
+          ].includes(opts.environment)
+        ) {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (!opts.environment || !["development", "production"].includes(opts.environment)) {
if (
!opts.environment ||
![
"development",
"production",
].includes(opts.environment)
) {
🧰 Tools
🪛 eslint

[error] 356-356: A linebreak is required after '['.

(array-bracket-newline)

[error] 356-356: There should be a linebreak after this element.

(array-element-newline)

[error] 356-356: A linebreak is required before ']'.

(array-bracket-newline)

🪛 GitHub Check: Lint Code Base

[failure] 356-356:
A linebreak is required after '['

[failure] 356-356:
There should be a linebreak after this element

[failure] 356-356:
A linebreak is required before ']'

throw new Error("Project environment is required. Supported environments are development and production.");
}
this.environment = opts.environment

Check failure on line 359 in packages/sdk/src/server/index.ts

View workflow job for this annotation

GitHub Actions / Lint Code Base 

Missing semicolon
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add missing semicolon after assignment

The statement this.environment = opts.environment at line 359 is missing a semicolon. Depending on the project's linting rules, omitting semicolons may cause issues.

Apply this diff to fix the issue:

-        this.environment = opts.environment
+        this.environment = opts.environment;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
this.environment = opts.environment
this.environment = opts.environment;
🧰 Tools
🪛 eslint

[error] 359-360: Missing semicolon.

(@typescript-eslint/semi)

🪛 GitHub Check: Lint Code Base

[failure] 359-359:
Missing semicolon


this.projectId = opts.projectId;
if (!this.projectId) {
Expand Down
Loading