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

Refactor: add a private constructor for connector #1115

Closed

Conversation

dolmen
Copy link
Contributor

@dolmen dolmen commented May 27, 2020

Description

This is a minor refactoring that will allow to prepare more things when the Connector is created and do less on each Connect. Those changes will be proposed in further PRs (spoil: I intend to move some code from handleParams once #1111 is merged).

Checklist

  • Code compiles correctly
  • Created tests which fail without the change (if possible)
  • All tests passing
  • Extended the README / documentation, if necessary
  • Added myself / the copyright holder to the AUTHORS file

This will allow to prepare more things in the constructor and do less on
each Connect.
@julienschmidt julienschmidt added this to the v1.6.0 milestone Jun 1, 2020
@julienschmidt
Copy link
Member

The change as it currently is, is a bit useless.
But #1111 was merged. Feel free to start moving code ;)

@dolmen
Copy link
Contributor Author

dolmen commented Jun 2, 2020

I agree this minor refactoring is a "a bit useless", but at least it should not be controversial and is a good base for further changes (which might be more controversial and might require multiple tries that would all start with this change).

@julienschmidt
Copy link
Member

And there is nothing wrong with including this very minor change in the maybe controversial change.
This PR is not an improvement.

@julienschmidt julienschmidt removed this from the v1.6.0 milestone Jun 2, 2020
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.

None yet

2 participants