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

consider renaming the raw document update types to include some hints of what kind of document objects they work with #86

Closed
heckj opened this issue Dec 1, 2023 · 4 comments
Assignees
Labels
enhancement New feature or request

Comments

@heckj
Copy link
Collaborator

heckj commented Dec 1, 2023

from @jessegrosjean in Discord chat:

since ObjIds are not typed, it might make API easier to read if actions are named based on type they can happen to. Right now some are, some are not. And it gets a little confusing since you have to switch on prop for some to determine what kind of object you are working with. So maybe more consistent would be action names:

  • MapPut
  • MapDelete
  • MapConflict
  • ListSplice
  • ListPut
  • ListConflict
  • TextSplice
  • TextMarks
  • CounterIncrement
@heckj heckj added the enhancement New feature or request label Dec 1, 2023
@jessegrosjean
Copy link
Contributor

The context is that I'm using patch events to update my internal application model.

I know the schema that my application model has encoded to automerge, but I don't really know what automerge will be sending back to me (maybe old app version, maybe buggy peer, etc). So when processing patches I want types to be pretty clear ... so that I can be sure I'm patching the correct application model object.

@heckj heckj linked a pull request May 7, 2024 that will close this issue
2 tasks
@heckj heckj self-assigned this May 7, 2024
@jessegrosjean
Copy link
Contributor

jessegrosjean commented May 8, 2024

Here's code that I couldn't get to post in Discord:

func getInMap(_ mapId: ObjId, key: String) -> Value?

Here's what all three version look like in use:

doc.get(obj: mapId, key: "name")
doc.getInMap(mapId, key: "name")
doc.mapGet(obj: mapId, key: "name")

And I would like to highlight that I really have no strong opinion here, except that some hinting of type in naming would be very welcome. I change my mind every few seconds on the exact form that should take.

@jessegrosjean
Copy link
Contributor

jessegrosjean commented May 8, 2024

Or if sticking with mapGet form what about hiding the obj param like:

func mapGet(_ mapId: ObjId, key: String) -> Value?

Then it looks like:

doc.mapGet(mapId, key: "name")

@heckj
Copy link
Collaborator Author

heckj commented Jul 13, 2024

After trying it out and living with the revised names a bit, while keeping the original functional, I think the maintenance overhead of the revised naming wasn't likely worth the effort, especially with nobody else really chiming in. So for this, I'm going to close it down with an explicit intention to keep the API as it is, even with the potential confusion points.

@heckj heckj closed this as completed Jul 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants