Skip to content

Conversation

@dimovpetar
Copy link
Member

No description provided.

@dimovpetar dimovpetar marked this pull request as draft October 21, 2025 11:50
@dimovpetar dimovpetar force-pushed the feat_run_manifest_validation branch 4 times, most recently from c0f9c6b to 85d48a7 Compare October 28, 2025 13:56
@dimovpetar dimovpetar force-pushed the feat_run_manifest_validation branch 2 times, most recently from 9ee721e to b1ed71b Compare October 30, 2025 08:30
@coveralls
Copy link

coveralls commented Oct 30, 2025

Pull Request Test Coverage Report for Build 19034241365

Details

  • 89 of 93 (95.7%) changed or added relevant lines in 5 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.5%) to 81.894%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/tools/run_manifest_validation/runValidation.ts 48 52 92.31%
Totals Coverage Status
Change from base Build 19030937854: 0.5%
Covered Lines: 1090
Relevant Lines: 1236

💛 - Coveralls

@dimovpetar dimovpetar marked this pull request as ready for review October 30, 2025 14:16
@alexandar-mitsev
Copy link
Member

Looks good to me

@dimovpetar dimovpetar force-pushed the feat_run_manifest_validation branch from 6ee3a30 to 0f4d9df Compare November 3, 2025 12:11

const log = getLogger("utils:ui5Manifest");

const LATEST_SCHEMA_URL = "https://raw.githubusercontent.com/SAP/ui5-manifest/main/schema.json";
Copy link
Member

Choose a reason for hiding this comment

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

Have you considered adding the @ui5/manifest npm package as a dependency instead of dynamically fetching the schema?

I guess the benefit of this approach is that the user will always validate against the latest version of the schema. But actually this could be a drawback as well since the MCP Server will become less deterministic.

A manifest.json that validates fine today might show errors tomorrow if a newly published schema makes respective changes. We should at least clarify with the maintainers of ui5-manifest whether pointing directly to the main branch (usually used for development) is fine here.

}, async ({manifestPath}) => {
log.info(`Running manifest validation on ${manifestPath}...`);

const result = await runValidation(manifestPath);
Copy link
Member

Choose a reason for hiding this comment

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

Just like for the create tool, please normalize the path using the Context:

const normalizedBasePath = await context.normalizePath(params.basePath);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants