-
Notifications
You must be signed in to change notification settings - Fork 211
Master -> dev #7169
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
Master -> dev #7169
Conversation
[PROD HOTFIX] - add wipro group for TG community
add debug logs
fixmore wipro groups
Clean up logs / PM-2525
Prod hotfix - Get off of v3jwt and fix MM related issues
HOTFIX - Only look at latest submissions for provisional score ranking
Additional ranking fix
[PROD] - AI Workflows MVP
Fix review opportunity filtering
PROD - PM-3080 / PS-476
…odes_only [PROD] - UTM register btn
| - run: | ||
| name: App npm install | ||
| command: npm install | ||
| command: npm ci |
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.
[correctness]
Switching from npm install to npm ci is a good practice for CI environments as it ensures a clean install based on the lock file. However, ensure that the package-lock.json is up-to-date and committed to the repository to avoid inconsistencies.
| --url https://circleci.com/api/v2/project/github/$CIRCLE_PROJECT_USERNAME/$CIRCLE_PROJECT_REPONAME/pipeline \ | ||
| --header "Circle-Token: ${CIRCLE_TOKEN}" \ | ||
| --header 'content-type: application/json' \ | ||
| --data '{"branch":"'"$CIRCLE_BRANCH"'","parameters":{"run_smoketesting":true , "run_performancetesting":false, "run_basedeployment": false}}' |
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.
[❗❗ security]
Ensure that the Circle-Token used in the curl command is securely managed and not exposed in logs or version control. Consider using environment variables or CircleCI's secure contexts to manage sensitive information.
| RUN npm config set unsafe-perm true | ||
| RUN git config --global url."https://git@".insteadOf git:// | ||
| RUN npm install | ||
| RUN npm ci |
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.
[correctness]
Switching from npm install to npm ci is a good change for ensuring a clean and consistent install of dependencies based on the package-lock.json. However, ensure that the package-lock.json is up-to-date and committed to the repository to avoid potential issues with missing or outdated dependencies.
|
|
||
| // handle values that might contain '=' | ||
| const cookieValue = decodeURIComponent(cookieStr.split('=').slice(1).join('=')); | ||
| return JSON.parse(cookieValue); |
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.
[❗❗ correctness]
Consider adding error handling for JSON.parse to handle cases where the cookie value is not a valid JSON string. This could prevent unexpected runtime errors.
| * @param url - The base URL to append parameters to | ||
| * @returns URL with UTM parameters appended, or original URL if no cookie exists | ||
| */ | ||
| export function appendUtmParamsToUrl(url, defaultParams = {}) { |
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.
[correctness]
The function appendUtmParamsToUrl should validate that url is a valid URL string before proceeding. This would prevent potential exceptions when constructing the URL object.
| } | ||
|
|
||
| try { | ||
| const urlObj = new URL(url, window.location.origin); |
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.
[correctness]
Using window.location.origin as a base for the URL constructor may not be appropriate if the url parameter is an absolute URL. Consider checking if url is absolute before using window.location.origin.
No description provided.