fix: Updated the build Scripts , and workflows to successful build.#62
fix: Updated the build Scripts , and workflows to successful build.#62drtechie merged 3 commits intoPSMRI:developfrom
Conversation
WalkthroughThis pull request updates several configuration and workflow files to improve the CI/CD process. GitHub Actions workflows are modified to handle submodules automatically by renaming and adding steps for manual updating. The submodule settings in the repository are adjusted, including updating URLs and commit references. Build configurations have been enhanced in Angular and package scripts by adding a prebuild check for the environment file and updating dependencies. A minor service change redirects an HTTP call to a new endpoint. Changes
Sequence Diagram(s)sequenceDiagram
participant CI as CI Pipeline
participant Repo as Repository
participant Prebuild as Prebuild Script
participant Angular as Angular Build
CI->>Repo: Checkout code with submodules
Note over CI,Repo: Automatic and manual submodule updates applied
CI->>Prebuild: Run prebuild step (check/create environment file)
Prebuild-->>CI: Prebuild completion confirmation
CI->>Angular: Execute ng build command (build-ci)
Angular-->>CI: Return build results
Assessment against linked issues
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
.github/workflows/package.yml (1)
47-47: Good update to actions/upload-artifact@v3Updating to the newer version of the upload-artifact action is beneficial for improved features and security. There's a trailing space at the end of this line that should be removed for better code cleanliness.
- uses: actions/upload-artifact@v3 + uses: actions/upload-artifact@v3🧰 Tools
🪛 actionlint (1.7.4)
47-47: the runner of "actions/upload-artifact@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 YAMLlint (1.35.1)
[error] 47-47: trailing spaces
(trailing-spaces)
.github/workflows/build-on-pull-request.yml (1)
11-16: Good improvement to submodule handling in checkoutRenaming the step and adding the
submodules: trueparameter ensures that submodules are automatically checked out during the build process, which aligns with the PR objectives for proper submodule handling.- submodules: true + submodules: true🧰 Tools
🪛 actionlint (1.7.4)
12-12: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 YAMLlint (1.35.1)
[error] 16-16: trailing spaces
(trailing-spaces)
package.json (1)
7-8: Prebuild Script for Environment Setup.
The introduction of theprebuildscript, which checks for the existence ofsrc/environments/environment.tsand creates it if missing, is a useful safeguard to prevent build failures. As a nitpick, please verify cross-platform compatibility—this shell syntax works on Unix-like systems but may need adjustments for Windows environments (or a note in the README to advise using a compatible shell).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.github/workflows/build-on-pull-request.yml(1 hunks).github/workflows/package.yml(1 hunks).gitmodules(1 hunks)Common-UI(1 hunks)angular.json(1 hunks)package.json(3 hunks)scripts/ci-prebuild.js(1 hunks)src/app/app-modules/core/services/auth.service.ts(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/package.yml
47-47: the runner of "actions/upload-artifact@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
.github/workflows/build-on-pull-request.yml
12-12: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 YAMLlint (1.35.1)
.github/workflows/package.yml
[error] 47-47: trailing spaces
(trailing-spaces)
.github/workflows/build-on-pull-request.yml
[error] 16-16: trailing spaces
(trailing-spaces)
🔇 Additional comments (11)
.gitmodules (1)
3-3: Submodule URL Updated CorrectlyThe removal of the
.gitsuffix aligns with the updated GitHub workflows and submodule handling, ensuring that the proper repository version is checked out during builds.Common-UI (1)
1-1: Submodule Commit Update Validated.The submodule commit has been updated to
71a223686e9f4bae4e425be286d362a23998cb2a. Ensure that this update has been tested so that it integrates smoothly with the main repository.scripts/ci-prebuild.js (1)
46-46: Good addition of SESSION_STORAGE_ENC_KEY to default environment valuesThis addition allows for configuration of the session storage encryption key through environment variables, which is good practice for security-related settings across different deployment environments.
src/app/app-modules/core/services/auth.service.ts (1)
51-51: Fixed endpoint reference for getUserDetails methodThe change correctly updates the endpoint to use
getSessionExistsURLinstead of the previous incorrect reference, aligning with the PR objectives. The method now properly checks for existing sessions..github/workflows/build-on-pull-request.yml (1)
18-21: Good addition of manual submodule update stepAdding this fallback step ensures that submodules are properly initialized and updated even if the automatic checkout doesn't fully succeed. This provides redundancy and makes the build process more robust.
angular.json (2)
56-62: Enhancement in Production Build Configuration.
The addition of"optimization": trueand the new"fileReplacements"array ensures that during production builds thesrc/environments/environment.tsfile is correctly replaced withsrc/environments/environment.prod.ts. This aligns well with Angular best practices for environment-specific builds and ensures proper configuration for production deployments.
65-70: Streamlining the Development Configuration.
In the development configuration, removal of the"namedChunks"property (with retention of"sourceMap": true) simplifies the build output while still enabling debugging support. Ensure that the removal of named chunks does not affect any downstream tooling that might rely on them.package.json (4)
14-17: Consistent Prebuild Integration in Build Commands.
Integrating the environment check directly into thebuild-dev,build-prod,build-ci, andbuild-testscripts ensures that all build configurations are protected against missing environment files. The additional steps (likechmod +x ./scripts/*.jsin the CI build) further enhance reliability.
44-44: New Dependency: ng2-smart-table.
The addition ofng2-smart-tableappears intended to enhance table functionalities. Please verify that its version (^1.2.1) is fully compatible with your Angular 16 setup and does not introduce any conflicts with existing libraries.
46-46: New Dependency: node-sass.
node-sasshas been added for styling needs. As many projects are transitioning tosass(ordart-sass), please confirm thatnode-sassis the preferred choice here and that it works seamlessly with the Angular build process, especially given evolving ecosystem trends.
78-78: Updated TypeScript Version Specification.
Changing the TypeScript version specifier from a tilde (~5.1.3) to a caret (^5.1.3) is a good move for accommodating minor version updates. Please ensure that all the related tooling and type definitions are tested against this broader version range.
|
Made changes to .eslintrc.json and updated the workflows to now implement npm run build-ci and removed prebuild script |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
.github/workflows/build-on-pull-request.yml (3)
11-17: Review of Checkout Submodules StepThe "Checkout code with submodules" step is now using
actions/checkout@v3with thesubmodules: trueparameter, which correctly enables automatic submodule checkout. However, note that the static analysis tool (actionlint) flags a potential issue with the runner ofactions/checkout@v3being outdated. Please verify whether a newer version (or additional configuration) is recommended by GitHub Actions. Also, line 16 contains trailing spaces that should be removed to conform to YAML formatting guidelines.🧰 Tools
🪛 actionlint (1.7.4)
12-12: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 YAMLlint (1.35.1)
[error] 16-16: trailing spaces
(trailing-spaces)
18-21: Review of Manual Submodule Update StepA new step "Update submodules manually (if needed)" has been added. While explicitly running
git submodule initandgit submodule update --recursivecan serve as a fallback, it might be redundant given that the checkout step already checks out submodules. Please confirm if this redundancy is intentional (perhaps for additional safety) or if the step can be removed to simplify the workflow.
42-42: Review of Build Command UpdateThe build command is now updated to
npm run build-ci. Ensure that thebuild-ciscript is properly defined in yourpackage.jsonand matches the intended build process. This update seems aligned with the overall goal for a refined CI build, but a quick verification might help avoid unexpected issues during subsequent builds..eslintrc.json (1)
35-36: Updated Angular ESLint Rules for TypeScript
Two new rules have been added in the TypeScript overrides:
"@angular-eslint/template/click-events-have-key-events": 0disables the rule to possibly reduce false positives in click event handling."eqeqeq": 1sets the loose equality check rule to a warning level rather than enforcing strict equality.Please verify that having a warning-level for
eqeqeqin TypeScript while the JavaScript override enforces strict equality ("error", "always") is intentional. Consistency across the codebase (or a clear rationale for differing levels) may help avoid unintended discrepancies.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.eslintrc.json(2 hunks).github/workflows/build-on-pull-request.yml(2 hunks)package.json(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/build-on-pull-request.yml
12-12: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 YAMLlint (1.35.1)
.github/workflows/build-on-pull-request.yml
[error] 16-16: trailing spaces
(trailing-spaces)
🔇 Additional comments (5)
.eslintrc.json (5)
8-8: Simplified TypeScript File Pattern
The override now cleanly specifies the files array as["*.ts"], which improves readability and maintainability.
40-45: Enhanced HTML Override Configuration
The HTML override has been streamlined by condensing the file pattern to a single line (["*.html"]) and extending the configurations to include both accessibility (plugin:@angular-eslint/template/accessibility) and Prettier integration (plugin:prettier/recommended). This approach helps ensure that HTML templates are checked for accessibility and style consistency.
46-53: Prettier Rule for HTML Templates
The rule"prettier/prettier": ["error", { "parser": "angular" }]is added within the HTML override. This configuration enforces formatting standards using Angular’s parser, which should aid in maintaining consistent code style in your templates.
56-69: Strict Linting Settings for JavaScript/JSX
The override for JavaScript and JSX files now includes:
- A Node environment setup via
"env": { "node": true }.- Extended base configuration with
"eslint:recommended".- Parser options adjusted for ECMAScript 2020 and module source type.
- Rules that enforce a warning for unused variables, disallow console statements, and strictly enforce equality via
"eqeqeq": ["error", "always"].This setup promotes robust linting in JS/JSX files. Ensure that the stricter
eqeqeqrule here, compared to the lenient one in the TypeScript configuration, is by design.
1-73: Overall ESLint Configuration Integration
The overall structure shows a coherent strategy for applying specific linting rules based on file types (TypeScript, HTML, JavaScript/JSX). This targeted approach improves the development workflow by ensuring that each file type adheres to the relevant style and consistency standards.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (10)
.github/workflows/build-on-pull-request.yml (3)
16-16: Remove Trailing Spaces
There are trailing spaces at the end of the line settingsubmodules: true. These extra spaces can sometimes lead to YAML parsing issues or style inconsistencies.- submodules: true + submodules: true🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 16-16: trailing spaces
(trailing-spaces)
18-21: Verify the Need for Manual Submodule Update
The new step to manually initialize and update submodules might be redundant since the checkout step now automatically includes submodules. Please verify if this extra step is necessary or if it can be removed to simplify the workflow.
23-32: Consider Upgrading Setup Actions for Consistency
The JDK and Node.js setup steps are currently usingactions/setup-java@v2andactions/setup-node@v2. For consistency with other workflows (which have already moved to newer versions) and to benefit from the latest improvements, consider upgrading these to@v4if possible.🧰 Tools
🪛 actionlint (1.7.4)
24-24: the runner of "actions/setup-java@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
30-30: the runner of "actions/setup-node@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 YAMLlint (1.35.1)
[error] 28-28: trailing spaces
(trailing-spaces)
.github/workflows/package-prod.yml (4)
20-20: Remove Trailing Spaces on Line 20
Trailing spaces can lead to formatting issues. Please remove the extra spaces on line 20.- uses: actions/checkout@v4 + uses: actions/checkout@v4🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 20-20: trailing spaces
(trailing-spaces)
25-25: Fix Indentation in Java Setup
The static analysis tool indicates a wrong indentation level on line 25 (expected 8 spaces but found 10). Please adjust the indentation in this section for consistency and to avoid YAML parsing warnings.- java-version: '17' + java-version: '17'🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 25-25: wrong indentation: expected 8 but found 10
(indentation)
27-28: Remove Trailing Spaces
Trailing spaces are present on lines 27 and 28. Removing these will help maintain consistent formatting.- - +(Ensure that lines 27 and 28 do not contain any extra spaces)
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 27-27: trailing spaces
(trailing-spaces)
[error] 28-28: trailing spaces
(trailing-spaces)
46-46: Remove Trailing Spaces in Artifact Upload Section
There are trailing spaces at the end of the line in the upload-artifact step. Removing these will ensure cleaner YAML.- uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@v4🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 46-46: trailing spaces
(trailing-spaces)
.github/workflows/package.yml (3)
18-20: Remove Trailing Spaces on Checkout Step
Static analysis has flagged trailing spaces on line 20. Please remove these extra spaces to adhere to YAML style guidelines.- uses: actions/checkout@v4 + uses: actions/checkout@v4🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 20-20: trailing spaces
(trailing-spaces)
24-26: Correct Indentation in Java Setup
There is an indentation warning on line 25 (expected 8 spaces but found 10). Please adjust the indentation for consistency with YAML standards.- with: - java-version: '17' - distribution: 'adopt' + with: + java-version: '17' + distribution: 'adopt'🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 25-25: wrong indentation: expected 8 but found 10
(indentation)
44-46: Remove Trailing Spaces in Artifact Upload
Trailing spaces are detected on line 45. Removing these will clean up the formatting.- uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@v4🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 45-45: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
.github/workflows/build-on-pull-request.yml(2 hunks).github/workflows/package-prod.yml(2 hunks).github/workflows/package.yml(2 hunks).gitignore(1 hunks)package.json(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- .gitignore
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
🧰 Additional context used
🪛 YAMLlint (1.35.1)
.github/workflows/package-prod.yml
[error] 20-20: trailing spaces
(trailing-spaces)
[warning] 25-25: wrong indentation: expected 8 but found 10
(indentation)
[error] 27-27: trailing spaces
(trailing-spaces)
[error] 28-28: trailing spaces
(trailing-spaces)
[error] 46-46: trailing spaces
(trailing-spaces)
.github/workflows/build-on-pull-request.yml
[error] 16-16: trailing spaces
(trailing-spaces)
.github/workflows/package.yml
[error] 20-20: trailing spaces
(trailing-spaces)
[warning] 25-25: wrong indentation: expected 8 but found 10
(indentation)
[error] 45-45: trailing spaces
(trailing-spaces)
🪛 actionlint (1.7.4)
.github/workflows/build-on-pull-request.yml
12-12: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
| - name: Checkout code with submodules | ||
| uses: actions/checkout@v3 | ||
| with: | ||
| ref: ${{ github.event.pull_request.head.ref }} | ||
| repository: ${{ github.event.pull_request.head.repo.full_name }} | ||
| submodules: true | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Update Checkout Action Version
The checkout step is still using actions/checkout@v3. For improved stability and to align with other workflows in the repository, please update this to actions/checkout@v4.
- - name: Checkout code with submodules
- uses: actions/checkout@v3
- with:
- ref: ${{ github.event.pull_request.head.ref }}
- repository: ${{ github.event.pull_request.head.repo.full_name }}
- submodules: true
+ - name: Checkout code with submodules
+ uses: actions/checkout@v4
+ with:
+ ref: ${{ github.event.pull_request.head.ref }}
+ repository: ${{ github.event.pull_request.head.repo.full_name }}
+ submodules: true📝 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.
| - name: Checkout code with submodules | |
| uses: actions/checkout@v3 | |
| with: | |
| ref: ${{ github.event.pull_request.head.ref }} | |
| repository: ${{ github.event.pull_request.head.repo.full_name }} | |
| submodules: true | |
| - name: Checkout code with submodules | |
| uses: actions/checkout@v4 | |
| with: | |
| ref: ${{ github.event.pull_request.head.ref }} | |
| repository: ${{ github.event.pull_request.head.repo.full_name }} | |
| submodules: true |
🧰 Tools
🪛 actionlint (1.7.4)
12-12: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 YAMLlint (1.35.1)
[error] 16-16: trailing spaces
(trailing-spaces)
2 participants
📋 Description
JIRA ID: N/A
GitHub Issue: Fixes PSMRI/AMRIT#55
Changes Made :
Updated the submodule to the latest commit.
GitHub Actions: Build on Pull Request
.github/workflows/build-on-pull-request.ymlto properly handle submodules during the build.GitHub Actions: Package Workflow
actions/upload-artifact@v2withactions/upload-artifact@v3in.github/workflows/package.yml.Prebuild Command:
package.jsonto introduce aprebuildcommand that sets up the environment before running the build process.Consistent Build Handling:
build-cibuild-prodbuild-devbuild-testProduction File Replacement:
angular.jsonto implementfileReplacementsfor production builds.fixed Minor issue of wrong reference
getSessionExistsURLin src/app/app-modules/core/services/auth.service.ts✅ Type of Change
ℹ️ Additional Information
Testing :
Screenshots
Summary by CodeRabbit
Chores
.gitignoreto excludesrc/environments/environment.ci.tsfrom version control.Refactor