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

Refactor automerge url utilities #205

Merged
merged 10 commits into from
Oct 31, 2023
Merged

Refactor automerge url utilities #205

merged 10 commits into from
Oct 31, 2023

Conversation

HerbCaudill
Copy link
Collaborator

  • renames DocUrl.ts to AutomergeUrl.ts
  • adds a set of basic tests for generateAutomergeUrl, stringifyAutomergeUrl, parseAutomergeUrl, and isValidAutomergeUrl
  • rewords some docs for clarity
  • refactors the utility functions

The only change in the API is to allow passing a document ID directly to stringifyAutomergeUrl, in addition to passing it as a property. So these are both valid and return the same result:

stringifyAutomergeUrl('4NMNnkMhL8jXrdJ9jamS58PAVdXu')
stringifyAutomergeUrl({ documentId: '4NMNnkMhL8jXrdJ9jamS58PAVdXu' })

I understand that the long-term plan is to support passing other things like heads or branch IDs in addition to document IDs. Presumably the primary use case will continue to be passing in a simple ID, and I think it's reasonable to make that as straightforward as possible for users.

@HerbCaudill HerbCaudill requested a review from pvh October 17, 2023 14:02
@HerbCaudill HerbCaudill force-pushed the refactor-automerge-url-utilities branch from 5a107ff to c3bc264 Compare October 30, 2023 12:04
@pvh pvh merged commit c3bc264 into main Oct 31, 2023
1 check passed
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