search

benthayer / git-gud

Wanna git gud? Then get git-gud, and git gud at git!
MIT License
401 stars 42 forks source link

Implement file_operator content #301

Closed sahansk2 closed 3 years ago

sahansk2 commented 3 years ago

Fixes #288

Replaces #289 with tests for all operating systems. https://github.com/sahansk2/git-gud/actions CircleCI build error because there's no CircleCI config.yml

sahansk2 commented 3 years ago

@benthayer , this is ready for review. Also, it's probably worth enabling GitHub actions workflows to run in pull requests.

sahansk2 commented 3 years ago

@benthayer , the matrix strategy is properly set up, and I added further modifications so that we can see all occurring errors across all OS's, even if the other OS's failed, and even if the previous (relevant) steps failed, since we don't necessarily want to have multiple commits just to fix flake8, which is the test that's run last.

See here: https://github.com/sahansk2/git-gud/runs/1024141817?check_suite_focus=true I cancelled the workflow, but notice how even though pytest for ubuntu-latest failed, flake8 was run anyway. Everything still comes up as an error, which is good.

So, this PR is ready for review again.

benthayer commented 3 years ago

@sahansk2 Can you update this so that flake8 only runs once? For now, it's fine that the windows test uses bash, but we should fix that at some point (can you add an issue?). Also can you put all of the testing commits on another branch? I'd like to merge into master with that first, then get to reviewing this PR. Once we merge that branch into master, then it will be appropriate to merge master into this branch and do a review

sahansk2 commented 3 years ago

Caching of the commit content doesn't actually seem to be necessary. I've added a commit to show this is the case.

sahansk2 commented 3 years ago

@benthayer This is ready for review. Could you comment on what needs to be done in order for this to be merged?

sahansk2 commented 3 years ago

This looks good, I just added subdirectory tests since they were unused.

benthayer commented 3 years ago

Great, will check it out within a day or two

On Sun, Sep 6, 2020, 2:36 PM Sahan Kumarasinghe notifications@github.com wrote:

This looks good, I just added subdirectory tests since they were unused.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/benthayer/git-gud/pull/301#issuecomment-687880325, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABORCY5RZ4LURST3F3VXA2LSEPQC7ANCNFSM4QJ7B23A .