fix : updated the scripts and workflows and .eslintrc.json so build passes successfully. - #46
Conversation
WalkthroughThis pull request makes configuration updates across various project files. The ESLint settings are modified to streamline TypeScript and HTML rules and add a new override for JavaScript/JSX. The GitHub Actions workflows in multiple YAML files have been updated to use newer action versions and revised build commands, including replacing the build commands with Changes
Sequence Diagram(s)sequenceDiagram
participant CI as "CI Pipeline"
participant Env as "Environment Checker"
participant Prebuild as "CI Prebuild Script"
participant Build as "Build Process"
CI->>Env: Check for "src/environments/environment.ts"
alt File Missing
Env->>CI: Create file using "touch"
else File Exists
Env->>CI: Confirm existence
end
CI->>Prebuild: Execute ci-prebuild.js
Prebuild-->>CI: Set environment variables (incl. SESSION_STORAGE_ENC_KEY)
CI->>Build: Run build command ("npm run build-ci")
Build-->>CI: Return build output
Assessment against linked issues
Possibly related PRs
Suggested reviewers
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 (
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (8)
.github/workflows/build-on-pull-request.yml (1)
17-22: Setup Java Action Upgrade & Formatting
Switching toactions/setup-java@v4is well executed. Please verify that the specified distribution ('adopt') remains valid for v4. Also, note that YAMLlint flagged trailing spaces on line 22—removing these will improve file consistency.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 22-22: trailing spaces
(trailing-spaces)
.github/workflows/package.yml (3)
18-20: Upgraded Checkout Action & Trailing Spaces Cleanup
The checkout step is now usingactions/checkout@v4, which is appropriate. Please also remove any trailing spaces (e.g., on line 20) for cleaner YAML formatting.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 20-20: trailing spaces
(trailing-spaces)
22-26: Setup Java Step Formatting
Updating toactions/setup-java@v4is a solid improvement. There appears to be extra indentation on line 25—adjusting the indentation and removing any trailing spaces will help maintain standard YAML style.🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 25-25: wrong indentation: expected 8 but found 10
(indentation)
33-40: Updated Build Command & Formatting
The updated build commandnpm run build-cireflects the new build process. Please check for and remove any trailing spaces (notably on line 40) to satisfy YAML linting requirements.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 40-40: trailing spaces
(trailing-spaces)
.github/workflows/package-prod.yml (4)
1-1: YAML Formatting – Excess Spaces
The workflow name in line 1 (name: Package Prod) contains extra spaces after the colon. Please adjust to a consistent format (e.g.,name: Package Prod).🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 1-1: too many spaces after colon
(colons)
18-20: Upgraded Checkout Action
The checkout step now usesactions/checkout@v4, which is a welcome update. Additionally, remove any trailing whitespace on line 20.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 20-20: trailing spaces
(trailing-spaces)
22-28: Set Up Java – Indentation & Trailing Spaces
The Java setup usesactions/setup-java@v4correctly; however, please ensure that thewithblock maintains consistent indentation (note the extra spaces on line 25) and remove trailing spaces from lines 27–28.🧰 Tools
🪛 YAMLlint (1.35.1)
[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)
35-42: Updated Build Command Consistency
The build command change tonpm run build-ciis in line with the new standard. Please remove any trailing spaces present on line 42.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 35-35: 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 (10)
.eslintrc.json(3 hunks).github/workflows/build-on-pull-request.yml(2 hunks).github/workflows/package-prod.yml(3 hunks).github/workflows/package.yml(2 hunks).gitignore(1 hunks).gitmodules(1 hunks)Common-UI(1 hunks)angular.json(1 hunks)package.json(1 hunks)scripts/ci-prebuild.js(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.35.1)
.github/workflows/package-prod.yml
[warning] 1-1: too many spaces after colon
(colons)
[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] 43-43: trailing spaces
(trailing-spaces)
[error] 44-44: trailing spaces
(trailing-spaces)
.github/workflows/build-on-pull-request.yml
[error] 22-22: 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] 40-40: trailing spaces
(trailing-spaces)
[error] 41-41: trailing spaces
(trailing-spaces)
[error] 43-43: trailing spaces
(trailing-spaces)
🔇 Additional comments (16)
Common-UI (1)
1-1: Confirm Submodule Commit Hash UpdateThe submodule commit has been updated to
881c2339634ed4a292d271d52c4365af496405fd. Please ensure that this new commit has been thoroughly tested and that its changes do not inadvertently affect the build process.scripts/ci-prebuild.js (1)
45-45:Details
❓ Verification inconclusive
New environment variable added for session storage encryption
A new environment variable
SESSION_STORAGE_ENC_KEYhas been added with an empty default value. This appears to be related to encryption of browser session storage data.While the implementation is straightforward, ensure that this key is properly populated in CI environments to avoid security risks. Empty encryption keys could potentially compromise the security of stored data.
🏁 Script executed:
#!/bin/bash # Check if SESSION_STORAGE_ENC_KEY is being set in other files or CI configuration grep -r "SESSION_STORAGE_ENC_KEY" --include="*.json" --include="*.yml" --include="*.yaml" .Length of output: 91
Action Required: Verify CI Configuration for Session Storage Encryption Key
The new environment variable
SESSION_STORAGE_ENC_KEYis defined inscripts/ci-prebuild.js(line 45) with an empty default value. Our initial grep search in JSON, YAML (and similar configuration files) did not return any instances of this key, which suggests it might be intended to be set externally (e.g., via environment variables or secret management in CI).To ensure stored session data is properly secured, please manually verify that a proper, non-empty encryption key is being provided in your CI environment settings. If this key is meant to be configured via an external file (such as a
.envfile) or a CI secret configuration, double-check that the configuration is correctly updated..gitignore (1)
46-46: Environment file correctly excluded from version controlAdding
environment.ci.tsto .gitignore is appropriate as it's a generated file from the ci-prebuild script that may contain sensitive information..eslintrc.json (3)
8-9: Format simplification for TypeScript files configurationThe TypeScript files array has been simplified to a single line, and the Prettier plugin has been removed from the extends array. This change streamlines the configuration.
Also applies to: 13-14
40-41: Format simplification for HTML files configurationThe HTML files array has been simplified to a single line, improving readability.
55-69: Added JavaScript/JSX linting configurationNew ESLint configuration for JavaScript and JSX files has been added with appropriate rules, which will help maintain code quality in these file types.
The configuration enforces good practices like:
- Warning for unused variables
- Prohibiting console statements
- Requiring strict equality checks
This will help catch issues during the build process, aligning with the PR objectives to fix build issues.
.gitmodules (1)
3-3: Standardized submodule URL formatThe URL for the Common-UI submodule has been updated to remove the
.gitsuffix, which standardizes the URL format while maintaining the same functionality.This change aligns with the PR objective of updating the submodule references.
angular.json (1)
56-62: Production Build Configuration – File Replacements Added
The addition of the"fileReplacements"property in the production configuration correctly maps the non-production environment file to"src/environments/environment.prod.ts". This change ensures that environment-specific settings are applied during production builds..github/workflows/build-on-pull-request.yml (3)
12-12: Checkout Action Upgrade
Upgrading fromactions/checkout@v3toactions/checkout@v4is a positive change and aligns with current best practices.
23-26: Node.js Setup Action Upgrade
The update toactions/setup-node@v4is correctly applied and helps maintain consistency across workflows.
29-34: Revised Build Command
Changing the build command tonpm run build-ciensures consistency with the new build scripts..github/workflows/package.yml (2)
28-31: Node.js Setup Action
The node setup now usesactions/setup-node@v4, which is correctly configured.🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 31-31: wrong indentation: expected 8 but found 10
(indentation)
[error] 31-31: trailing spaces
(trailing-spaces)
41-49: Artifact Upload Step Update
Upgrading toactions/upload-artifact@v4is properly implemented. The configuration for uploading the WAR file looks correct.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 41-41: trailing spaces
(trailing-spaces)
[error] 43-43: trailing spaces
(trailing-spaces)
[error] 48-48: trailing spaces
(trailing-spaces)
.github/workflows/package-prod.yml (2)
29-32: Node.js Setup Action
The node setup section is correctly updated to useactions/setup-node@v4.🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 32-32: wrong indentation: expected 8 but found 10
(indentation)
45-50: Artifact Upload Step
The artifact upload is correctly configured usingactions/upload-artifact@v4, with proper artifact naming and path settings.package.json (1)
13-16: Robust Environment File Check in Build Scripts
The updated build scripts for"build-dev","build-prod","build-ci", and"build-test"now include a check to ensure thatsrc/environments/environment.tsexists before the build process runs. This proactive safeguard should prevent build failures related to a missing file.
| "@typescript-eslint/no-explicit-any": 0, | ||
| "@typescript-eslint/no-empty-function": 0, | ||
| "@typescript-eslint/no-unused-vars": 0, | ||
| "@angular-eslint/template/click-events-have-key-events": 0, |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Accessibility rule disabled
The rule requiring click events to have corresponding key events has been disabled, which may impact accessibility for keyboard users.
Consider addressing the underlying accessibility issues rather than disabling the rule. This approach would ensure the application remains accessible to keyboard users, including those using screen readers or those who cannot use a mouse.
🏁 Script executed:
#!/bin/bash
# Find components with click events that may need keyboard event handlers
grep -r "(click)" --include="*.html" src/
Length of output: 6103
Accessibility: Review and Update Keyboard Event Handling
The ESLint rule "@angular-eslint/template/click-events-have-key-events": 0 in your configuration is currently disabled. The grep search confirms that many components (e.g., in app-header, camera-dialog, common-dialog, etc.) have (click) handlers without obvious corresponding keyboard event handlers. This can significantly affect keyboard accessibility for users.
Please consider:
- Reviewing Interactive Elements: Identify non-semantic or custom interactive elements that rely solely on click events.
- Implementing Keyboard Events: Add relevant keyboard event handlers (such as
(keydown.enter)or(keyup.space)) to these elements where appropriate. - Re-Enabling the ESLint Rule: Once the accessibility issues are addressed, re-enable the rule to help maintain these standards across the codebase.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/workflows/package.yml (2)
20-20: Formatting: Remove Trailing Spaces
The YAML linter has flagged trailing spaces on several lines (lines 20, 40, 41, and 43). Cleaning these up will help maintain a clean configuration file.Also applies to: 40-40, 41-41, 43-43
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 20-20: trailing spaces
(trailing-spaces)
25-25: Formatting: Fix Indentation in Java Setup
YAML lint indicates a wrong indentation on line 25 (expected 8 spaces, found 10). Adjusting the indentation will improve readability and prevent potential parsing issues.🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 25-25: wrong indentation: expected 8 but found 10
(indentation)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/package-prod.yml(0 hunks).github/workflows/package.yml(2 hunks)
💤 Files with no reviewable changes (1)
- .github/workflows/package-prod.yml
🧰 Additional context used
🪛 YAMLlint (1.35.1)
.github/workflows/package.yml
[error] 20-20: trailing spaces
(trailing-spaces)
[warning] 25-25: wrong indentation: expected 8 but found 10
(indentation)
[error] 40-40: trailing spaces
(trailing-spaces)
[error] 41-41: trailing spaces
(trailing-spaces)
[error] 43-43: trailing spaces
(trailing-spaces)
🔇 Additional comments (6)
.github/workflows/package.yml (6)
19-19: Update: Upgraded Checkout Action to v4
The checkout action has been updated to v4, which aligns with current best practices and should improve performance and feature support.
23-23: Update: Upgraded Setup-Java Action to v4
The Java setup action is now using v4. Please double-check that the provided Java version and distribution settings meet your build requirements.
29-29: Update: Upgraded Setup-Node.js Action to v4
The Node.js setup action has been upgraded to v4 and is configured for node-version 18. Verify that this version is consistent with the rest of your development environment.
39-39: Change: Updated Build Command
The build command has been changed to usenpm run build-ci. Ensure that this command is correctly defined in your package scripts and that it fulfills all CI build requirements.
45-45: Update: Upgraded Upload-Artifact Action to v4
The artifact upload action has been updated to v4, which should provide enhanced functionality and bug fixes.
48-48: Change: Updated Artifact Path
The artifact path is now set todist/scheduler-ui/scheduler-ui.war, ensuring the WAR file is correctly referenced. Confirm that your build process outputs the WAR file to this updated location.
There was a problem hiding this comment.
@Zapper9982 we have to add the submodules. Guess it's missing.
There was a problem hiding this comment.
Yes , let me fix that up one second !
|
@drtechie Updated it to handle submodules |
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
.github/workflows/build-on-pull-request.yml (3)
11-17: Updated Checkout Step with Submodule Support and Trailing Whitespace Issue
The upgraded checkout action (actions/checkout@v4) now correctly enables submodule checkout (line 16). Please remove the trailing spaces on line 16 to address YAML lint warnings.- submodules: true + submodules: true🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 16-16: trailing spaces
(trailing-spaces)
18-21: Added Manual Submodule Update Step
A new step to manually initialize and update submodules has been added. While this provides a safety net, please verify whether this is redundant given that the checkout step now enables submodules by default.
23-28: JDK Setup Step Updated and Trailing Spaces Noted
The JDK setup now usesactions/setup-java@v4with the correct Java version and distribution. However, a trailing whitespace issue is detected on line 28. Please remove the extra spaces to clean up the YAML.- +🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 28-28: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/build-on-pull-request.yml(2 hunks)
🧰 Additional context used
🪛 YAMLlint (1.35.1)
.github/workflows/build-on-pull-request.yml
[error] 16-16: trailing spaces
(trailing-spaces)
[error] 28-28: trailing spaces
(trailing-spaces)
🔇 Additional comments (2)
.github/workflows/build-on-pull-request.yml (2)
29-32: Updated Node.js Setup Step
The Node.js setup usesactions/setup-node@v4with node version 18 as expected. This update aligns well with the overall improvement of the build process.
34-41: Build Command Update for Consistency
The build step now invokesnpm run build-ci, which is consistent with the updated build process across workflows. All installation and configuration commands prior to this step appear in order.
📋 Description
JIRA ID:
GitHub Issue: Fixes PSMRI/AMRIT#50
Changes Made :
Updated the submodule to the latest commit.
Updated the
.eslintrc.jsonto handle js/jsx as ci-prebuild is a .js fileGitHub Actions: Build on Pull Request
.github/workflows/build-on-pull-request.ymlto properly handle submodules during the build.GitHub Actions: Package Workflow
.github/workflows/. upgrade all to latets versions .Consistent Build Handling:
-- environment is first touched , and then the the command are run.
build-cibuild-prodbuild-devbuild-test✅ Type of Change
Screenshots
Summary by CodeRabbit
Chores
SESSION_STORAGE_ENC_KEYfor enhanced configuration management..gitignoreforsrc/environments/environment.ci.ts.Refactor
.gitsuffix.New Features