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

Setup Pre-Commit Hook to Run Tests & Dogma #10

Open
nelsonic opened this issue Sep 11, 2017 · 6 comments
Open

Setup Pre-Commit Hook to Run Tests & Dogma #10

nelsonic opened this issue Sep 11, 2017 · 6 comments

Comments

@nelsonic
Copy link
Member

as a developer I don't want to have to remember to run the linter (dogma)
each time I commit my code ...
I want it to run automatically!
see: https://github.com/dwyl/learn-pre-commit

e.g: https://travis-ci.org/dwyl/hits-elixir/builds/274319451#L689
image
where the tests pass but the "dogma" code style fails...! 🙄

@nelsonic
Copy link
Member Author

Hoping someone else has discovered an elegant way of doing this in without resorting to node.js ... dwyl/learn-pre-commit#6 (comment)

@Cleop
Copy link
Member

Cleop commented Feb 16, 2018

How about this one https://github.com/dwyl/elixir-pre-commit @nelsonic?

Currently in the process of adding it although encountering an error with mix credo that I'm having to look into.

image
image

@Cleop Cleop self-assigned this Feb 16, 2018
Cleop added a commit that referenced this issue Feb 16, 2018
@nelsonic
Copy link
Member Author

@Cleop yeah, I just wanted help from someone to implement it. 😉

@Cleop
Copy link
Member

Cleop commented Feb 16, 2018

Came across these re broken credo on the credo repo:
rrrene/credo#463
image
rrrene/credo#496
image

Changing the credo line as the second comment suggests got it running successfully on mix credo but failing when I try to do a commit
Pass:
image
Fail with no further explanation:
image

More on the same here: rrrene/credo#469

Maybe I'm best off adding my feedback to the credo repo too, despite the fact it's supposedly been fixed there... looks like some other people are still experiencing the problem:
image

@nelsonic @SimonLab - what do you think? Just want to make sure I've not overlooked anything?

Cleop added a commit that referenced this issue Feb 16, 2018
@Cleop
Copy link
Member

Cleop commented Feb 19, 2018

I've asked directly on the credo repo here on an update/ advice: rrrene/credo#495 (comment)

@Cleop
Copy link
Member

Cleop commented Feb 27, 2018

Response from @rrene shared these links:

Credo fails with an exit status != 0 if it shows any issues. This enables shell based pipeline workflows (e.g. on CI systems) which test Credo compliance.

These are the scores of exit statuses that credo emits based on any shortcomings in your code:
image

Therefore the reason the pre-commit is failing is because we're not exiting with an exit status of 0 because of the amendments that could be made to the code.

So to solve this issue we need to resolve the credo suggestions to give an exit status of 0.

nelsonic added a commit that referenced this issue Feb 27, 2018
…e-commit hooks which are now handled in Elixir-land! #10)
nelsonic added a commit that referenced this issue Feb 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants