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: Using local sflight clone instead of git dependency #883

Merged
merged 7 commits into from
Nov 6, 2024

Conversation

danjoa
Copy link
Contributor

@danjoa danjoa commented Nov 5, 2024

Simplifies our setup, and speeds up pipelines

@danjoa danjoa changed the title Using local sflight clone instead of git dependency chore: Using local sflight clone instead of git dependency Nov 5, 2024
@BobdenOs
Copy link
Contributor

BobdenOs commented Nov 6, 2024

My main concern is that copying sflight and bookshop into the repository is that they will go out of sync and not just that people have already modified the bookshop model. Which causes trouble where sometimes the main state of the bookshop does not work while the cds-dbs state of the bookshop is passing without any issues.

If using sub-modules would provide the same performance benefit as it would be git cloned from github.com anyway. That would be my preference as it doesn't allow people to modify the copy and will make it straight forward to keep the state up to date.

@danjoa
Copy link
Contributor Author

danjoa commented Nov 6, 2024

Agreed, but we anyways have to find better ways to deal with that ... i.e. we need some umbrella-kind-of setup like cap/dev available externally in public GitHub. That also is the preliminary conclusion from respective considerations with Daniel around Incidents Mgmt and the Calesi repos (which all live in public Github).

This PR is a near-term measure to fix problems we have with always having a duplicate install of SFlight in cap/dev today, as well as the problem that Steffen encountered recently when he had to revert a change he thought to do locally in SFlight which broke tests in cds-dbs. → I'd propose to merge that for now and follow up on the bigger thing near term.

@danjoa danjoa enabled auto-merge (squash) November 6, 2024 13:55
@danjoa danjoa merged commit 8bb8206 into main Nov 6, 2024
4 checks passed
@danjoa danjoa deleted the local-sflight-clone branch November 6, 2024 14:22
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