Interval tree initial documentation and structures defintions #13
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
AlexandruCihodaru
requested review from
alsrdn,
andreeaflorescu,
gsserge and
jiangliu
gsserge
reviewed
Feb 7, 2022
gsserge
reviewed
Feb 7, 2022
gsserge
reviewed
Feb 7, 2022
gsserge
reviewed
Feb 7, 2022
gsserge
reviewed
Feb 7, 2022
gsserge
reviewed
Feb 7, 2022
AlexandruCihodaru
force-pushed
the
interval_tree_doc
branch
3 times, most recently
from
556c3d7 to
81a6a2a
Compare
studychao
reviewed
Feb 8, 2022
Collaborator
studychao
left a comment
studychao
left a comment
There was a problem hiding this comment.
Hi Alex, thanks for your work and here is my comments 😄
AlexandruCihodaru
force-pushed
the
interval_tree_doc
branch
from
81a6a2a to
695d974
Compare
gsserge
previously approved these changes
Feb 9, 2022
studychao
previously approved these changes
Feb 9, 2022
AlexandruCihodaru
changed the title
Closed
andreeaflorescu
requested changes
Feb 9, 2022
Member
andreeaflorescu
left a comment
andreeaflorescu
left a comment
There was a problem hiding this comment.
Chatted with @AlexandruCihodaru offline. Before merging the PR:
- update the allocation_example.png to make it more explicit in terms of state transition and the operations that are generating the change
- include in the design document the design choices and the reasoning behind them: we want to use an interval tree for performance reasons, what are these reasons? where might it be used?
- remove the structure definitions from the design document and replace them with text where that makes sense. This helps with having mostly up to date documentation even when implementation details are changing.
For later (i.e. other PRs):
- examples are better to be placed in src/lib.rs because they are validated at runtime and they don't get so easily outdated.
AlexandruCihodaru
dismissed stale reviews from studychao and gsserge
via
9bcff4e
AlexandruCihodaru
force-pushed
the
interval_tree_doc
branch
from
695d974 to
9bcff4e
Compare
andreeaflorescu
previously approved these changes
Feb 10, 2022
Member
andreeaflorescu
left a comment
andreeaflorescu
left a comment
There was a problem hiding this comment.
There are quite a few typos and other spell errors in the README. Can you please address them in a future PR?
gsserge
previously approved these changes
Feb 10, 2022
* Add initial structures definitions. Signed-off-by: AlexandruCihodaru <[email protected]> Co-authored-by: Liu Jiang <[email protected]>
* Started the design and documentation of IntervalTree component of the AddressAllocator. Signed-off-by: AlexandruCihodaru <[email protected]> Co-authored-by: Liu Jiang <[email protected]>
AlexandruCihodaru
dismissed stale reviews from gsserge and andreeaflorescu
via
9a402d4
AlexandruCihodaru
force-pushed
the
interval_tree_doc
branch
from
9bcff4e to
9a402d4
Compare
AlexandruCihodaru
requested review from
andreeaflorescu,
gsserge and
studychao
gsserge
previously approved these changes
Feb 10, 2022
studychao
reviewed
Feb 10, 2022
Signed-off-by: AlexandruCihodaru <[email protected]>
AlexandruCihodaru
force-pushed
the
interval_tree_doc
branch
from
9a402d4 to
67661c6
Compare
studychao
approved these changes
Feb 10, 2022
gsserge
approved these changes
Feb 10, 2022
andreeaflorescu
approved these changes
Feb 10, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
IntervalTree.IntervalTreeallocation engine.Created a new PR as suggested by @gsserge here