Skip to content

Remove circular dependencies #662

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 4 commits into from
Jan 15, 2021

Conversation

bigmontz
Copy link
Contributor

@bigmontz bigmontz commented Jan 13, 2021

Remove circular dependencies to decouple modules.

Circular dependencies overview:

circular

@bigmontz bigmontz force-pushed the 4.3-remove-cyclic-dependencies branch from 2f38157 to a6c9fe0 Compare January 14, 2021 10:53
@bigmontz bigmontz marked this pull request as ready for review January 14, 2021 10:55
@bigmontz bigmontz requested a review from 2hdddg January 14, 2021 10:55
Copy link
Contributor

@2hdddg 2hdddg left a comment

Choose a reason for hiding this comment

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

Looks good! I assume temporal functions has been more or less copied ?

@@ -30,9 +29,9 @@ function encode (str) {
}

function decode (buffer, length) {
if (buffer instanceof NodeBuffer) {
if (Object.prototype.hasOwnProperty.call(buffer, '_buffer')) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like there should be a decode function on NodeBuffer and CombinedBuffer

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, It should be a method present in all the buffers versions, the issue is way it's done vary a bit between buffer types, I could not figure out a way to do it properly.

@bigmontz bigmontz merged commit 06aed3d into neo4j:4.3 Jan 15, 2021
@bigmontz bigmontz deleted the 4.3-remove-cyclic-dependencies branch January 15, 2021 13:57
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants