Skip to content

Conversation

injectives
Copy link
Contributor

@injectives injectives commented Apr 29, 2022

Bookmark objects should represent singular bookmark values. Multiple bookmarks will be represented as a set of bookmarks (Set<Bookmark>).

The main API updates are highlighted below.

Bookmark updates:

  • value() - new method
  • values() - deprecated
  • isEmpty() - deprecated

Session and AsyncSession updates:

  • Set<Bookmark> lastBookmarks() - new method
  • Bookmark lastBookmark() - deprecated

ReactiveSession updates:

  • Set<Bookmark> lastBookmarks() - new method
  • Bookmark lastBookmark() - deprecated (has not been released)

@injectives injectives force-pushed the feature/bookmarks branch 3 times, most recently from cd48d0b to 8cf2572 Compare May 4, 2022 20:37
@injectives injectives changed the title Update bookmarks Update bookmarks API May 4, 2022
@injectives injectives force-pushed the feature/bookmarks branch 2 times, most recently from 09b7beb to 090a891 Compare May 4, 2022 20:52
`Bookmark` objects should represent singular bookmark values. Multiple bookmarks will be represented as a set of bookmarks (`Set<Bookmark>`).

The main API updates are highlighted below.

`Bookmark` updates:
- `value()` - new method
- `values()` - deprecated
- `isEmpty()` - deprecated

`Session` and `AsyncSession` updates:
- `Set<Bookmark> lastBookmarks()` - new method
- `Bookmark lastBookmark()` - deprecated

`ReactiveSession` updates:
- `Set<Bookmark> lastBookmarks()` - new method
- `Bookmark lastBookmark()` - deprecated (has not been released)
@injectives injectives force-pushed the feature/bookmarks branch from 090a891 to 4202244 Compare May 4, 2022 20:58
@injectives injectives requested a review from michael-simons May 5, 2022 08:14
Copy link
Contributor

@michael-simons michael-simons left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really not a fan about returning multiple bookmarks, I still don't understand the technical reasons behind it.

Copy link
Contributor

@michael-simons michael-simons left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed in person.

@injectives injectives merged commit c521c51 into neo4j:5.0 May 12, 2022
@injectives injectives deleted the feature/bookmarks branch May 12, 2022 16:14
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