Skip to content

Conversation

@dominique-vassard
Copy link
Contributor

@dominique-vassard dominique-vassard commented Jan 30, 2019

Hi @mschae
I've implemented Bolt V3. (Issues #26)

I think though that a refactoring will be needed, the 3 versions and mixed in all files and it's hard to understand how a version is different from another. It relies on doc and on the fact that user knows what he's doing. Not really the best, and in fact the code is not self explanatory anymore.
Before tackling this issue, I wait to know what's going on with the bolt_sips - boltex separation: 2 projects or 1?

Thank you

@coveralls
Copy link

coveralls commented Jan 30, 2019

Coverage Status

Coverage increased (+1.1%) to 96.335% when pulling 347d700 on dominique-vassard:bolt_v3 into acf71d9 on mschae:master.

@dominique-vassard
Copy link
Contributor Author

Finally everything is green :)

@mschae
Copy link
Owner

mschae commented Mar 18, 2019

So sorry for dropping the ball here, is this good to merge?

@@ -1,5 +1,9 @@
# Changelog

## v 0.6.0
Copy link
Owner

Choose a reason for hiding this comment

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

I think this merits a new major version.

# ]

# Supervisor.init(children, strategy: :one_for_one)
# end
Copy link
Owner

Choose a reason for hiding this comment

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

Let's delete everything that's commented out.

def set(version) do
Agent.update(__MODULE__, fn _ -> version end)
end
end
Copy link
Owner

Choose a reason for hiding this comment

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

I'm not 100% but if I have a project and I connect to both v2 and v3 databases, wouldn't we still only have one instance of this agent and wouldn't it be confused?

I'd prefer introducing our own Connection struct that stores the connection (hence the PID of the tcp process) and the version used.

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.

3 participants