fix : made required changes so build succeeds#93
Conversation
WalkthroughThis pull request updates multiple GitHub Actions workflows by upgrading action versions (e.g., checkout, setup-java, setup-node, and upload-artifact) and removing specific npm install commands related to certain dependencies. It also updates environment file usage in Angular configurations and package build scripts by ensuring that the environment file exists before a build. Additional modifications include an updated subproject commit in Common-UI, a new entry in the .gitignore, added environment variables in a CI prebuild script, and a revised import path in an Angular component. These changes are made to improve the CI/CD pipeline consistency and maintain dependency configurations. Changes
Sequence Diagram(s)sequenceDiagram
participant Script
participant FileSystem
participant BuildProcess
Script->>FileSystem: Check if "src/environments/environment.ts" exists
alt File is missing
FileSystem-->>Script: Not found
Script->>FileSystem: Create file using "touch"
else File exists
FileSystem-->>Script: File present
end
Script->>BuildProcess: Execute Angular build command
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
🪧 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 (
|
| import { ConfirmationService } from 'src/app/app-modules/core/services'; | ||
| import { SetLanguageComponent } from 'src/app/app-modules/core/components/set-language.component'; | ||
| import { environment } from 'src/environments/environment.development'; | ||
| import { environment } from 'src/environments/environment.dev'; |
There was a problem hiding this comment.
souldn't we use environment.ts here?
Angular anyways replaces the file.
There was a problem hiding this comment.
Ohh yeah @drtechie , seems valid let me update it !
![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 (4)
.github/workflows/package.yml (3)
29-32: Indentation Issue in Java Setup
Thejava-versionanddistributionfields under thewith:key are indented with 10 spaces rather than the expected 8. Please adjust the indentation to align with YAML standards.Consider applying this diff:
- java-version: '17' - distribution: 'adopt' + java-version: '17' + distribution: 'adopt'🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 31-31: wrong indentation: expected 8 but found 10
(indentation)
40-45: Build Command Update and Trailing Whitespace
The build step now usesnpm run build-ci, which aligns with the updated build process. However, static analysis has flagged trailing whitespace (notably on the line withnpm run build-ci). Removing these extra spaces will help avoid YAML lint warnings.For example, ensure the command line reads exactly:
- npm run build-ci␣␣ + npm run build-ci
47-55: Upload Artifact Step and Trailing Whitespace
The upload step has been updated to useactions/upload-artifact@v4and the artifact path has been changed todist/hwc-ui/hwc-ui.war, which is correct. Please also remove any trailing whitespace (notably on the line specifying the path) as flagged by static analysis.A diff suggestion to remove trailing spaces on the artifact path line:
- path: dist/hwc-ui/hwc-ui.war␣ + path: dist/hwc-ui/hwc-ui.war🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 47-47: trailing spaces
(trailing-spaces)
[error] 49-49: trailing spaces
(trailing-spaces)
[error] 54-54: trailing spaces
(trailing-spaces)
.github/workflows/build-on-pull-request.yml (1)
22-27: Trailing Whitespace in JDK Setup Section
While the Java setup update toactions/setup-java@v4is correctly implemented, static analysis identified trailing whitespace on line 27 (the blank or spacing line following the distribution setting). Please remove the extra spaces to conform to YAML formatting guidelines.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 27-27: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/build-on-pull-request.yml(3 hunks).github/workflows/package-prod.yml(0 hunks).github/workflows/package.yml(3 hunks)
💤 Files with no reviewable changes (1)
- .github/workflows/package-prod.yml
🧰 Additional context used
🪛 YAMLlint (1.35.1)
.github/workflows/build-on-pull-request.yml
[error] 27-27: trailing spaces
(trailing-spaces)
.github/workflows/package.yml
[warning] 31-31: wrong indentation: expected 8 but found 10
(indentation)
[error] 46-46: trailing spaces
(trailing-spaces)
[error] 47-47: trailing spaces
(trailing-spaces)
[error] 49-49: trailing spaces
(trailing-spaces)
[error] 54-54: trailing spaces
(trailing-spaces)
🔇 Additional comments (5)
.github/workflows/package.yml (2)
19-22: Update to Checkout Action Version
The checkout step now usesactions/checkout@v4with proper submodule settings and fetch depth. This update looks good.
35-37: Update to Node.js Setup
The Node.js step now correctly usesactions/setup-node@v4with Node version 18. This update is correct and aligns with the new standards.🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 37-37: wrong indentation: expected 8 but found 10
(indentation)
[error] 37-37: trailing spaces
(trailing-spaces)
.github/workflows/build-on-pull-request.yml (3)
11-17: Checkout Action Update Verification
The checkout step in this workflow now usesactions/checkout@v4along with proper reference extraction and submodule support. This change is implemented correctly.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 17-17: trailing spaces
(trailing-spaces)
28-32: Node.js Setup Update
The Node.js step correctly usesactions/setup-node@v4with Node version 18. This change is good as-is.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 32-32: trailing spaces
(trailing-spaces)
33-39: Build Command Consistency
The build step usesnpm run build-ci, which is consistent with the changes in the other workflow file and the updated build process. No issues are noted here.
|
@Zapper9982 there are conflicts in this PR. |
|
@drtechie Resolved the merge Conflict |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/workflows/package.yml (2)
29-32: Updated Setup Java Action Version & Fix YAML Indentation
The Java setup action has been updated toactions/setup-java@v4, ensuring modern configuration and performance improvements. However, YAMLlint has flagged an indentation issue on line 31 (expected 8 spaces but found 10). Please adjust the indentation for consistency.Example diff:
- java-version: '17' - distribution: 'adopt' + java-version: '17' + distribution: 'adopt'🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 31-31: wrong indentation: expected 8 but found 10
(indentation)
46-49: Remove Trailing Whitespace
Static analysis reports trailing whitespace on lines 46, 47, and 49. Removing these extraneous spaces will improve readability and maintain clean formatting.Example diff:
- npm run build-ci + npm run build-ci - - - name: Create WAR file + - name: Create WAR file - +Note: Ensure that only the unwanted whitespace is removed so that the intended YAML structure is preserved.
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 46-46: trailing spaces
(trailing-spaces)
[error] 47-47: trailing spaces
(trailing-spaces)
[error] 49-49: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/package.yml(3 hunks)Common-UI(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- Common-UI
🧰 Additional context used
🪛 YAMLlint (1.35.1)
.github/workflows/package.yml
[warning] 31-31: wrong indentation: expected 8 but found 10
(indentation)
[error] 46-46: trailing spaces
(trailing-spaces)
[error] 47-47: trailing spaces
(trailing-spaces)
[error] 49-49: trailing spaces
(trailing-spaces)
🔇 Additional comments (4)
.github/workflows/package.yml (4)
19-19: Updated Checkout Action Version
The checkout action is updated to useactions/checkout@v4, which provides improved performance and additional features. The current configuration correctly includes submodules and fetches the full history.
35-37: Updated Setup Node.js Action Version
The Node.js setup action is now usingactions/setup-node@v4, which aligns with expected updates. The configuration under thewithblock appears correct.🧰 Tools
🪛 YAMLlint (1.35.1)
[warning] 37-37: wrong indentation: expected 8 but found 10
(indentation)
[error] 37-37: trailing spaces
(trailing-spaces)
45-45: Revised Build Command
The build command has been updated tonpm run build-ci, ensuring that the CI-specific build process is executed instead of the previous generic build. Verify that the correspondingbuild-ciscript in yourpackage.jsonis correctly configured.
51-55: Updated Upload Artifact Action and Artifact Path
The upload artifact action now usesactions/upload-artifact@v4, and the artifact path has been changed todist/hwc-ui/hwc-ui.war. Confirm that this new path correctly points to the built artifact for your project.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.eslintrc.json (2)
38-38: Accessibility Override: Verify Intent of Disabling Click Events Key Events Rule
The rule"@angular-eslint/template/click-events-have-key-events": 0disables the accessibility check for key events on click events in Angular templates. Please ensure that this decision is intentional and that alternative measures are in place to guarantee adequate accessibility in these components.
60-74: New JavaScript/JSX ESLint Override: Check Consistency & Configuration
This override for*.jsand*.jsxfiles introduces a Node.js environment, sets the ECMAScript version to 2020, and enforces stricter linting rules (e.g.,"no-unused-vars": "warn","no-console": "error", and"eqeqeq": ["error", "always"]). Notice that the TypeScript override (lines 11–40) applies"eqeqeq": 1(a warning) while here it is an error. Please verify that this discrepancy is deliberate and aligns with your project’s linting and code quality standards.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.eslintrc.json(2 hunks)Common-UI(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- Common-UI
|
@drtechie noticed that the recent commit to the Common-UI causes some import problems, so reverted one commit back on Common-UI |
|
@Zapper9982, we will need the changes. Also looks like some environment properties are removed in this PR which was recently added in another PR. |
|
@drtechie I think you should close this Pull request , I'll pull my codebase once and raise another PR with the same , so we can cleanly update the changes ! |
|
@Zapper9982 you can delete the environment and language files from PR as those are required and not relevant to your PR. This will help to not remove wanted code. |
|
@helenKaryamsetty , @drtechie thanks for the help , I made the changes accordingly . |
|
3 participants
📋 Description
JIRA ID:
GitHub Issue: Fixes PSMRI/AMRIT#57
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
.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-testRemoved deprecated and unused nodemodules:
node-sassng2-smart-table✅ Type of Change
Screenshots
Summary by CodeRabbit
.gitignoreto exclude new environment configuration files.