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

Use project ID, not code, in CrdtMerge API #1169

Merged
merged 7 commits into from
Oct 28, 2024

Conversation

rmunn
Copy link
Contributor

@rmunn rmunn commented Oct 28, 2024

Fix #1168.

We now use project code and ID in root folder name, use "crdt" and "fw" inside root folder no matter what the project's actual code or name is.

We also use project ID rather than code in the API request parameters. So now instead of POSTing to /sync/?projectCode=sena-3, you would POST to /sync/?projectId=0ebc5976-058d-4447-aaa7-297f8569f968.

Copy link

github-actions bot commented Oct 28, 2024

C# Unit Tests

75 tests  ±0   75 ✅ ±0   5s ⏱️ ±0s
13 suites ±0    0 💤 ±0 
 1 files   ±0    0 ❌ ±0 

Results for commit cde1ab3. ± Comparison against base commit f72f3e0.

♻️ This comment has been updated with latest results.

@rmunn rmunn linked an issue Oct 28, 2024 that may be closed by this pull request
@rmunn rmunn changed the title Revisit filenames we use for CrdtMerge Use project ID, not code, in CrdtMerge API Oct 28, 2024
@rmunn
Copy link
Contributor Author

rmunn commented Oct 28, 2024

Draft PR for now. Once #1170 finishes passing tests, I'll merge it, then rebase this PR on top of it and then we'll be ready for review.

rmunn added 3 commits October 28, 2024 13:20
Use project code and ID in root folder name, use "crdt" and "fw" inside
root folder no matter what the project's actual code or name is.
Using the name "crdt" everywhere as a project name caused a couple of
caching issues in existing CRDT code, but the change is pretty easy.
@rmunn rmunn force-pushed the feat/crdtmerge-project-name-and-id branch from 987ac61 to a9aed68 Compare October 28, 2024 06:21
rmunn added 2 commits October 28, 2024 13:42
Details are a bit of a long story but has to do with how AsyncLocal
works, and which async variables are in scope at which point.
@rmunn rmunn marked this pull request as ready for review October 28, 2024 06:45
@rmunn rmunn requested a review from hahn-kev October 28, 2024 06:45
@rmunn rmunn merged commit baf6cd5 into develop Oct 28, 2024
13 checks passed
@rmunn rmunn deleted the feat/crdtmerge-project-name-and-id branch October 28, 2024 07:59
@rmunn rmunn self-assigned this Oct 28, 2024
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.

CrdtMerge: use project ID, not project code, in request
2 participants