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

Exercise connected_to and connects_to methods #34453

Merged

Conversation

bogdanvlviv
Copy link
Contributor

Since both methods are public API I think it makes sense to add these tests in order to prevent any regression in the behavior of those methods after the 6.0 release.

Exercise connected_to

  • Ensure that the method raises with both database and role arguments
  • Ensure that the method raises without database and role

Exercise connects_to

Related to #34052

@rails-bot
Copy link

r? @sgrif

(@rails-bot has picked a reviewer for you, use r? to override)

Copy link
Member

@eileencodes eileencodes left a comment

Choose a reason for hiding this comment

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

Can you squash your commits? Thanks!

@rafaelfranca
Copy link
Member

r? @eileencodes

@rails-bot rails-bot assigned eileencodes and unassigned sgrif Nov 15, 2018
Since both methods are public API I think it makes sense to add these tests
in order to prevent any regression in the behavior of those methods after the 6.0 release.

Exercise `connected_to`
  - Ensure that the method raises with both `database` and `role` arguments
  - Ensure that the method raises without `database` and `role`

Exercise `connects_to`
  - Ensure that the method returns an array of established connections(as mentioned
    in the docs of the method)

Related to rails#34052
@bogdanvlviv bogdanvlviv force-pushed the exercise-connected_to-and-connects_to branch from ba12ad5 to cf01da2 Compare November 15, 2018 09:50
@bogdanvlviv
Copy link
Contributor Author

I just squashed the commits into one. Thank you for the review!

Copy link
Contributor

@albertoalmagro albertoalmagro left a comment

Choose a reason for hiding this comment

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

@tenderlove tenderlove merged commit a95c2ec into rails:master Nov 19, 2018
@bogdanvlviv bogdanvlviv deleted the exercise-connected_to-and-connects_to branch November 19, 2018 21:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants