-
Notifications
You must be signed in to change notification settings - Fork 271
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 projects in transcripts #5078
Conversation
Fix recontextualization of project queries when using relativeTo
…paces for this any more
f86e36e
to
20b5e93
Compare
"tag": "Paragraph" | ||
} | ||
"hash": "#sg60bvjo91fsoo7pkh9gejbn0qgc95vra87ap6l5d35ri0lkaudl7bs12d71sf3fh6p23teemuor7mk1i9n567m50ibakcghjec5ajg", | ||
"readme": null |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this expected?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great catch! Forgot to update the api call to the projects endpoint.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed this and all the other api-* ones 👍🏼
@@ -12,6 +12,9 @@ GET /api/projects | |||
}, | |||
{ | |||
"projectName": "project-two" | |||
}, | |||
{ | |||
"projectName": "scratch" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This project is implictly created now.
|
||
foo/main> branch bar/topic | ||
bar/main> branch foo/main topic2 | ||
bar/main> branch foo/main /topic3 | ||
.> branch foo/main bar/topic4 | ||
|
||
.some.loose.code> branch foo/topic13 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These tests I just removed since we'll be removing loose code support very soon.
.> find-in somewhere bar | ||
scratch/main> find bar | ||
-- Shows UUIDs | ||
-- scratch/main> find.global bar |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This shows project UUIDs unfortunately, I'm hoping I can revive this behaviour correctly in the project-roots branch
unison-src/transcripts/fix1709.md
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I moved a few transcripts from transcripts-using-base
into transcripts
if they didn't actually depend on base because the extra history and global definitions on loose-code from the base transcript was complicating the output.
|
||
type A a b c d | ||
= B b | ||
= A a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not positive what determines the uuids used for determining constructor order tbh 🤷🏼♂️
@@ -1,9 +1,47 @@ | |||
# Tests for `move.namespace` | |||
|
|||
|
|||
## Moving the Root |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I moved the global root stuff to the top of this transcript, leaving it at the bottom meant it would have a bunch of non-deterministic .__projects.*
junk in it.
I'll end up rewriting this part in the project-roots branch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed the dependency on the base transcript for this one, since mixing the base transcript with projects stuff was causing UUIDs in the output.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whew! 😅
Nice work.
My eyes admittedly glazed over and didn't look super close at the multi-branch, "interesting" transcripts, but I flagged a couple of deletions that you've now fixed, thanks.
I switched from Chrome to Safari mid-review and that helped |
2b99f42
to
4b7cf99
Compare
Overview
Converts most transcripts to use projects rather than
.>
in loose code.The plan is to merge the project-centric transcripts to trunk so if I break things with the conversion to project roots (which removes loose code support) I can safely compare apples-to-apples rather than converting the tests and code in the same step.
Apologies, the diff view in github might seize up a bit 😓
I believe every transcript is converted except the following:
These transcripts I'm waiting for Mitchell to have a crack at first since they depend on
merge.old
:The following transcripts are only partially translated, they'll need changes from the project-roots branch to be fully translated:
These I haven't even attempted to translate since they'll require additional features based on the project root branch.
Implementation notes
The vast majority of changes are just swapping
.>
withscratch/main>
; but there are many transcripts which required other mechanical changes, specifically where subnamespaces were used. Most of these were replaced with either using project branches, or commands which work within a specific subnamespace instead.More significant changes are commented.
Test coverage
Yup it sure is.
Loose ends
Mentioned above; will be addressed in the project-roots branch.