Skip to content

cds add github-actions#29

Merged
swaldmann merged 19 commits into
mainfrom
gha
Feb 4, 2026
Merged

cds add github-actions#29
swaldmann merged 19 commits into
mainfrom
gha

Conversation

@swaldmann

Copy link
Copy Markdown
Contributor

No description provided.

Comment thread .github/workflows/cf.yaml Outdated
push:
branches:
- main
pull_request:

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Will remove this before merging so deployments are only done for pushes on main.

config.activate.on-profile: cloud
cds:
security.mock.users: {}
---

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

AMS requires us to disable mock users in the cloud… otherwise you get cryptic errors on Java server startup:
https://github.com/capire/xtravels-java/actions/runs/21216937849/job/61040618351#step:9:969

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for finding that. AFAIK that's actually a known issue. @BraunMatthias Isn't this fixed yet?

Comment thread .github/workflows/test.yaml Outdated
Comment on lines +1 to +32
name: Tests

on:
workflow_call:
workflow_dispatch:
pull_request:
merge_group:
push:
branches:
- main

permissions:
contents: read
packages: read

jobs:
tests:
runs-on: ubuntu-latest
steps:
- uses: actions/setup-node@v6
with:
node-version: 22
- uses: actions/checkout@v6
- run: npm install
env:
NODE_AUTH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
- uses: actions/setup-java@v5
with:
distribution: sapmachine
java-version: 21
cache: maven
- run: mvn test -B

@swaldmann swaldmann Feb 4, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This workflow and https://github.com/capire/xtravels-java/blob/main/.github/workflows/maven.yml seem quite redundant now. Imo we should keep this one, as it also runs tests and doesn't just build the project.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The other workflow also runs the tests, as test execution is included in mvn package :)
Also we shouldn't need to do an explicit npm install in a Java-based project.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Right, missed that in the other workflow's output. Still quite redundant, so should we just keep the maven.yaml one?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Just removed test.yaml in 1e51d35.

Revert security.mock.users configuration to include admin roles and change profile activation condition.
Comment thread .github/workflows/test.yaml Outdated
Comment on lines +1 to +32
name: Tests

on:
workflow_call:
workflow_dispatch:
pull_request:
merge_group:
push:
branches:
- main

permissions:
contents: read
packages: read

jobs:
tests:
runs-on: ubuntu-latest
steps:
- uses: actions/setup-node@v6
with:
node-version: 22
- uses: actions/checkout@v6
- run: npm install
env:
NODE_AUTH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
- uses: actions/setup-java@v5
with:
distribution: sapmachine
java-version: 21
cache: maven
- run: mvn test -B

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The other workflow also runs the tests, as test execution is included in mvn package :)
Also we shouldn't need to do an explicit npm install in a Java-based project.

Comment thread .npmrc Outdated
config.activate.on-profile: cloud
cds:
security.mock.users: {}
---

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for finding that. AFAIK that's actually a known issue. @BraunMatthias Isn't this fixed yet?

@swaldmann

Copy link
Copy Markdown
Contributor Author

Deployed app looks good on first sight:

Screenshot 2026-02-04 at 11 30 50

@swaldmann
swaldmann marked this pull request as ready for review February 4, 2026 10:51
@swaldmann

Copy link
Copy Markdown
Contributor Author

@beckermarc just sent the BTP links where the app is hosted to you via mail.

@swaldmann
swaldmann requested a review from beckermarc February 4, 2026 10:52
@beckermarc

Copy link
Copy Markdown
Contributor

Thanks a lot, very nice!

Bonus would now be: Add the same to XFlights and connect them to each other in Production deployments. For Staging we could use the mocked mode, as done right now.

Removed pull_request trigger from workflow.
@swaldmann
swaldmann enabled auto-merge (squash) February 4, 2026 12:21
@swaldmann
swaldmann merged commit 35d1a5e into main Feb 4, 2026
6 checks passed
@swaldmann
swaldmann deleted the gha branch February 4, 2026 12:24
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.

2 participants