search

openSUSE / gitarro

run all your test against a GitHub Pull request
https://opensuse.github.io/gitarro
MIT License
15 stars 20 forks source link

Remove jstatus class var #70

Closed MalloZup closed 7 years ago

MalloZup commented 7 years ago

fix #67 , and adress partially #66

MalloZup commented 7 years ago

RSPEC

7 examples, 0 failures

:sunglasses:

MalloZup commented 7 years ago

Imho, we have to assume that everybody run rake locally, and this is fine. (maybe we should add this in how-to-contribute workflow) https://github.com/openSUSE/gitarro/pull/72 (if we need credentials to much for unit-tests, then this is a rspec tests).

For unit-test, we can even think to mock the objects/function/data we need

juliogonzalez commented 7 years ago

I really like optimistic people :-)

But my experience tells me that we should run tests in all cases. People tends to forget about testing, specially after they make "minor" changes to a PR (and that's valid for me as well!)

That said, I agree with your reasoning that if we need credentials then it's not unit testing, so happy to approve & merge as long as we have an issue for this (doesn't need to be critical priority).

juliogonzalez commented 7 years ago

LGTM

MalloZup commented 7 years ago

issue for moving the tests here: https://github.com/openSUSE/gitarro/issues/74