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.

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

pull-request.yml updated #3289

Closed
wants to merge 1 commit into from
Closed

pull-request.yml updated #3289

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
25 changes: 16 additions & 9 deletions .github/workflows/pull-request.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
##############################################################################

Check warning on line 1 in .github/workflows/pull-request.yml

View workflow job for this annotation

GitHub Actions / Performs linting, formatting, type-checking, checking for different source and target branch 

File ignored by default.
##############################################################################
#
# NOTE!
Expand Down Expand Up @@ -421,22 +421,29 @@
name: Test Deployment to https://docs-admin.talawa.io
runs-on: ubuntu-latest
needs: [Docker-Start-Check, Start-App-Without-Docker]
# Run only if the develop-postgres branch and not dependabot
# Run only if the develop-postgres branch and not dependabot
if: ${{ github.actor != 'dependabot[bot]' && github.event.pull_request.base.ref == 'develop-postgres' }}
steps:
- uses: actions/checkout@v4
- uses: actions/setup-node@v4
- name: Checkout the Repository
uses: actions/checkout@v4

- name: Set up Node.js
uses: actions/setup-node@v4
with:
node-version: '22.x'
cache: yarn
cache-dependency-path: 'docs/'
# Run Docusaurus in the ./docs directory
node-version: '20.x'
Comment on lines +430 to +433
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

⚠️ Node.js version needs to be updated in all workflow files

While changing to Node.js 20.x is correct, the same update is needed in other workflow files that still reference version 22:

  • .github/workflows/push-deploy-website.yml: Line containing node-version: 22
  • .github/workflows/push.yml: Line containing node-version: [22.x]
🔗 Analysis chain

Good fix! Node.js version change is appropriate.

Changing from Node.js 22.x to 20.x is correct as:

  1. Node.js 22.x is not yet released
  2. Version 20.x is the current LTS release
  3. This aligns with the version used in other jobs

Let's verify the Node.js version compatibility with all dependencies:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check package.json for Node.js engine requirements
jq '.engines.node' docs/package.json 2>/dev/null || echo "No Node.js version specified in package.json"

# Check if any dependencies require Node.js >20.x
cd docs && npm ls 2>&1 | grep -i "required node version"

Length of output: 98

Script:

#!/bin/bash
# Find all package.json files and check their Node.js engine requirements
fd package.json --type f --exec sh -c 'echo "=== {} ==="; jq -r ".engines.node // \"No Node.js version specified\"" {}' \;

# Check for any potential Node.js version references in workflow files
rg "node-version:" .github/workflows/

Length of output: 764

🧰 Tools
🪛 yamllint (1.35.1)

[error] 433-433: trailing spaces

(trailing-spaces)


- name: Install dependencies
working-directory: ./docs
run: yarn install --frozen-lockfile
run: npm install

- name: Test building the website
working-directory: ./docs
run: yarn build
run: npm run build

- name: Check for Node.js memory issues
run: |
node --max-old-space-size=4096 ./node_modules/.bin/docusaurus build

Comment on lines +443 to +446
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

Remove redundant build step.

The additional build step with memory allocation is redundant as it runs after the previous build step. This could potentially:

  1. Overwrite the artifacts from the first build
  2. Unnecessarily increase the workflow execution time

Instead, modify the original build step to include the memory allocation:

  - name: Test building the website
  working-directory: ./docs
- run: npm run build
+ run: node --max-old-space-size=4096 ./node_modules/.bin/docusaurus build

Committable suggestion skipped: line range outside the PR's diff.


Check-Target-Branch:
if: ${{ github.actor != 'dependabot[bot]' }}
Expand Down
Loading