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

Jump to bottom

Add `perform_linear_number_of_queries` rspec matcher #39

Merged

palkan merged 8 commits into palkan:master from caalberts:add-buffer-option

Nov 17, 2020

Contributor

caalberts commented Oct 5, 2020 •

edited

perform_linear_number_of_queries takes a parameter slope to allow for linear number of queries.

What changes did you make? (overview)

Add a rspec matcher .perform_linear_number_of_queries(slope: 1) to allow for linear number of queries..

Is there anything you'd like reviewers to focus on?

Checklist

I've added tests for this change
I've added a Changelog entry
I've updated a documentation

caalberts commented

View reviewed changes

lib/n_plus_one_control/rspec/matcher.rb Outdated Show resolved Hide resolved

caalberts force-pushed the add-buffer-option branch from a451e61 to 28af9bc Compare

October 10, 2020 08:18

palkan reviewed

View reviewed changes

lib/n_plus_one_control/rspec/matchers/perform_linear_number_of_queries.rb Outdated Show resolved Hide resolved

caalberts force-pushed the add-buffer-option branch from 28af9bc to a49a22c Compare

October 20, 2020 12:56

caalberts commented

View reviewed changes

Contributor Author

caalberts left a comment

@palkan I made some changes to remove the intercept. If this approach looks good to you, I'll continue with the failure message and documentation.

lib/n_plus_one_control/rspec/matchers/perform_linear_number_of_queries.rb Show resolved Hide resolved

lib/n_plus_one_control/rspec/matchers/perform_linear_number_of_queries.rb Show resolved Hide resolved

palkan reviewed

View reviewed changes

Owner

palkan left a comment

@caalberts Looks good. Please, go ahead!

lib/n_plus_one_control/rspec/matchers/perform_linear_number_of_queries.rb Show resolved Hide resolved

lib/n_plus_one_control/rspec/matchers/perform_linear_number_of_queries.rb Show resolved Hide resolved

caalberts changed the title ~~Add with_buffer chain on rspec matcher~~ Add perform_linear_number_of_queries rspec matcher

caalberts force-pushed the add-buffer-option branch from 2ca5437 to 9d2a2a9 Compare

October 22, 2020 13:14

caalberts marked this pull request as ready for review

October 23, 2020 11:51

Contributor Author

caalberts commented Oct 23, 2020

@palkan Could you have another look please?

caalberts commented

View reviewed changes

lib/n_plus_one_control/rspec/matchers/perform_linear_number_of_queries.rb Show resolved Hide resolved

palkan requested changes

View reviewed changes

Owner

palkan left a comment

Looks good. Thanks!

Left some minor comments.

CHANGELOG.md Show resolved Hide resolved

lib/n_plus_one_control/rspec/matchers/perform_linear_number_of_queries.rb Show resolved Hide resolved

Contributor Author

caalberts commented Oct 30, 2020

@palkan I have made the additional changes. Could you take a look again please?

caalberts requested a review from palkan

October 31, 2020 01:35

palkan approved these changes

View reviewed changes

Owner

palkan left a comment

Great job 👍

Would you like to add Minitest support as well? Just asking, we can add it later.

And could you please rebase, there is a conflict.

caalberts added 8 commits

November 7, 2020 08:47


          Move files around

5c84642


          Add linear query matcher

89dc7de


          Add linear query failure message

2b4e0b9


          Add with_scale_factors chain

a638a6f


          Add with_warming_up chain and warmup block

f99677e


          Update changelog and readme

843c486

- rspec matcher `perform_linear_number_of_queries`


          Add url to changelog

e1ee9df


          Add chain :matching

c4b49f0

caalberts force-pushed the add-buffer-option branch from afc2fc3 to c4b49f0 Compare

November 7, 2020 00:48

Contributor Author

caalberts commented Nov 7, 2020

@palkan Rebased.

Would you like to add Minitest support as well? Just asking, we can add it later.

I'm not very familiar with Minitest. So maybe we can separate it into another PR?

Contributor Author

caalberts commented Nov 7, 2020

@palkan Rebased.

Would you like to add Minitest support as well? Just asking, we can add it later.

I'm not very familiar with Minitest. So maybe we can separate it into another PR?

I started a draft in caalberts#1. I'll move it into this repository after this PR is merged.

palkan merged commit aa1e54d into palkan:master

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment