Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Create sample CAP app #184

Merged
merged 34 commits into from
Oct 9, 2024
Merged

chore: Create sample CAP app #184

merged 34 commits into from
Oct 9, 2024

Conversation

ZhongpinWang
Copy link
Contributor

@ZhongpinWang ZhongpinWang commented Sep 30, 2024

Context

AI/gen-ai-hub-sdk-js-backlog#131.

Implemented a sample CAP application written in TypeScript to demonstrate the usage of SAP Cloud SDK for AI.

Definition of Done

  • [ ] Code is tested (Unit, E2E)
  • [ ] Error handling created / updated & covered by the tests above
  • Documentation updated
  • [ ] (Optional) Aligned changes with the Java SDK
  • [ ] (Optional) Release notes updated

@ZhongpinWang ZhongpinWang marked this pull request as draft September 30, 2024 13:54
@ZhongpinWang ZhongpinWang marked this pull request as ready for review October 1, 2024 09:30
Copy link

@kay-schmitteckert kay-schmitteckert left a comment

Choose a reason for hiding this comment

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

Thanks, looks good! Would be nice to also add a prettier config like


"prettier": {
        "arrowParens": "always",
        "tabWidth": 4,
        "semi": true,
        "singleQuote": false,
        "trailingComma": "none",
        "printWidth": 120
    }

sample-cap/README.md Outdated Show resolved Hide resolved
sample-cap/srv/orchestration/orchestration-service.ts Outdated Show resolved Hide resolved
sample-cap/srv/orchestration/orchestration-service.ts Outdated Show resolved Hide resolved
sample-cap/srv/foundation-models/azure-openai-service.cds Outdated Show resolved Hide resolved
@ZhongpinWang
Copy link
Contributor Author

Thanks, looks good! Would be nice to also add a prettier config like


"prettier": {
        "arrowParens": "always",
        "tabWidth": 4,
        "semi": true,
        "singleQuote": false,
        "trailingComma": "none",
        "printWidth": 120
    }

Thanks for your review! We have a prettier config in the root folder which applies to all packages.

package.json Outdated Show resolved Hide resolved
Copy link
Contributor

@marikaner marikaner left a comment

Choose a reason for hiding this comment

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

Looks good to me, but I left a few minor remarks.

.changeset/sweet-lies-confess.md Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
sample-cap/db/.gitkeep Outdated Show resolved Hide resolved
sample-cap/package.json Outdated Show resolved Hide resolved
sample-cap/srv/ai-api/ai-api-service.cds Outdated Show resolved Hide resolved
@ZhongpinWang ZhongpinWang changed the title feat: Create sample CAP app chore: Create sample CAP app Oct 7, 2024
package.json Outdated
"check:public-api": "pnpm -r check:public-api",
"check:deps": "pnpm -r -F !./tests/smoke-tests -F !./tests/schema-tests exec depcheck --ignores=\"nock,@jest/globals\" --quiet"
"check:deps": "pnpm -r -F !./tests/smoke-tests -F !./tests/schema-tests -F !./sample-cap exec depcheck --ignores=\"nock,@jest/globals\" --quiet"
Copy link
Contributor

Choose a reason for hiding this comment

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

[q] Why do we not check dependencies for the cap sample?

Copy link
Contributor

Choose a reason for hiding this comment

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

[pp] Just saw the other comment - what if we just ignore the cap types lib? Depchek supports the standard definitely typed packages, but not something like this...

Copy link
Contributor Author

@ZhongpinWang ZhongpinWang Oct 9, 2024

Choose a reason for hiding this comment

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

The thing is that, I also have to move @cap-js/cds-types back to the root package json, because if I put it in sample-cap and run pnpm install from root, it sometimes creates a symbolic link in the root node_models pointing to a directory which does not exist. (As you can see in the screenshot, sap__cds aka. ../@cap-js/cds-types simply does not exist. Maybe this path is hardcoded somehow, ignoring .pnpm)

image

This will also cause public-api check to fail due to this non-existing folder.

AND, this symbolic link only got created randomly. I also saw it sometimes in the pipeline causing an error for check:public-api, but if I click on re-run, it sometimes just turned green again..

So I now decide to move this dev deps in root and remove the filter in the dep check. This could be an issue of CAP.

@ZhongpinWang ZhongpinWang merged commit 2803146 into main Oct 9, 2024
9 checks passed
@ZhongpinWang ZhongpinWang deleted the feat-create-sample-cap-app branch October 9, 2024 15:14
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