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

Fix update hooks specs #74

Merged
merged 3 commits into from
Apr 10, 2017

Conversation

drebs
Copy link
Contributor

@drebs drebs commented Apr 4, 2017

The hook calls and the specs used different parameter names for pytest_benchmark_update_machine_info and pytest_benchmark_update_commit_info, and pytest complained and failed when trying to override those hooks.

These commits fix the specs by changing the parameters names. Another approach would be to maintain the specs and change the hook calls, but it made more sense to change the parameter names in specs to maintain coherence with other hooks and with the parts of the code that call these hooks.

This pull request is made on top of #73, so that one should be merged first.

@ionelmc
Copy link
Owner

ionelmc commented Apr 4, 2017

Hmmm, good catch!

I wonder how did it even work previously and why this still shows up the merged commits from the netrc PR ...

@drebs drebs force-pushed the fix-update-machine-info-hook-spec branch from e3c7719 to 9be023a Compare April 5, 2017 18:27
@codecov-io
Copy link

Codecov Report

Merging #74 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #74   +/-   ##
=======================================
  Coverage   77.71%   77.71%           
=======================================
  Files          16       16           
  Lines        1696     1696           
  Branches      303      303           
=======================================
  Hits         1318     1318           
  Misses        316      316           
  Partials       62       62

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bbb9406...9be023a. Read the comment docs.

@drebs
Copy link
Contributor Author

drebs commented Apr 5, 2017

I rebased and now only the 3 new commits are shown.

@ionelmc ionelmc merged commit d6f3066 into ionelmc:master Apr 10, 2017
leap-code-o-matic pushed a commit to leapcode/soledad that referenced this pull request Apr 20, 2017
There were some changes needed in pytest-benchmark so we could
successfully use it for soledad benchmarks graphing:

ionelmc/pytest-benchmark#73
ionelmc/pytest-benchmark#74
ionelmc/pytest-benchmark#75

The contributions were accepted but not released yet, so this commit
uses the code from upstream git repository's master branch.
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

3 participants