Open
Conversation
🦋 Changeset detectedLatest commit: 806fc63 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
cianmSAP
previously approved these changes
Feb 24, 2026
Contributor
cianmSAP
left a comment
cianmSAP
left a comment
There was a problem hiding this comment.
Updates look good
Have not tested
Changeset present
lfindlaysap
requested changes
Feb 24, 2026
packages/cf-deploy-config-writer/src/translations/cf-deploy-config-writer.i18n.json
Outdated
Show resolved
Hide resolved
packages/cf-deploy-config-writer/src/translations/cf-deploy-config-writer.i18n.json
Outdated
Show resolved
Hide resolved
lfindlaysap
requested changes
Feb 25, 2026
packages/cf-deploy-config-writer/src/translations/cf-deploy-config-writer.i18n.json
Outdated
Show resolved
Hide resolved
lfindlaysap
requested changes
Feb 25, 2026
packages/cf-deploy-config-writer/src/translations/cf-deploy-config-writer.i18n.json
Outdated
Show resolved
Hide resolved
- Add comprehensive unit tests for i18n module covering initialization and translation functions - Simplify i18n initialization error handling by removing verbose async wrapper - Errors are silently caught as writer still functions with fallback strings Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
Replace require() with dynamic import() to comply with @typescript-eslint/no-require-imports rule and remove unnecessary eslint-disable comment. Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
Priority 1 - JSDoc Standardization: - Add comprehensive @throws documentation across all source files - Enhance parameter descriptions with detailed context - Standardize capitalization and formatting in JSDoc comments - Document error conditions for key functions - Add missing JSDoc for private methods Priority 2 - Code Quality Improvements: - Unify toMtaModuleName implementations (use canonical version from utils.ts) - Fix error message i18n consistency (add error.noUI5AppFound key) - Document CAPConfig empty interface design rationale - Fix folder existence check in base-config.ts (use isMTAFound instead of fileExists) - Remove unused imports Files modified: - src/utils.ts: Enhanced JSDoc, improved toMtaModuleName documentation - src/mta-config/index.ts: Delegate to canonical toMtaModuleName, add @throws docs - src/mta-config/mta.ts: Standardize JSDoc for public/private methods - src/cf-writer/app-config.ts: Add @throws docs, use i18n for errors - src/cf-writer/base-config.ts: Fix validation logic, improve JSDoc - src/cf-writer/cap-config.ts: Add @throws documentation - src/types/index.ts: Document CAPConfig interface purpose - src/translations/cf-deploy-config-writer.i18n.json: Add error.noUI5AppFound key
Update test to match implementation changes where validation now uses fs.existsSync via isMTAFound() instead of mem-fs editor's exists method. Also update expected error message to reflect the specific mta.yaml check.
- Fix typos in README.md (ommit->omit, SAB->SAP, genereate->generate, serivce->service) - Consolidate duplicate example sections in README - Add comprehensive error handling documentation - Add CAP project considerations section - Add MTA module naming conversion documentation - Fix variable shadowing in code examples - Update test to use mem-fs-editor's exists() instead of fs.existsSync() - Restore compatibility with mem-fs virtual file system in base-config.ts - All tests passing with 100% coverage on modified files
…nfig-writer.i18n.json Co-authored-by: Louise Findlay <[email protected]>
…nfig-writer.i18n.json Co-authored-by: Louise Findlay <[email protected]>
longieirl
force-pushed
the
feature/code-quality-improvements-and-branch-protection
branch
from
efc2de4 to
c490901
Compare
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR contains comprehensive code quality improvements for the cf-deploy-config-writer package, including standardized JSDoc documentation, important bug fixes, and test updates to maintain compatibility with implementation changes.