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

Add 'docker run' example / incorrect docker result #183

Open
tommyalatalo opened this issue Feb 20, 2020 · 20 comments
Open

Add 'docker run' example / incorrect docker result #183

tommyalatalo opened this issue Feb 20, 2020 · 20 comments
Labels
documentation Improvements or additions to documentation DX DX improvements enhancement New feature or request

Comments

@tommyalatalo
Copy link

Could you add a "docker run ..." command example to the repository README.md docs?
I've been trying to run commitsar with a direct docker run command, and I think I have it running correctly with this: docker run --rm --name="commitsar" -w /src -v "$(pwd)":/src commitsar/commitsar:0.10.0

But the result is incorrect. I ran the above container from a repository with a master branch containing 71 commits, but commitsar just finds the latest commit and reports 1 of 1 commits are not conventional commit compliant. Why are the other 70 commits ignored?

@tommyalatalo tommyalatalo changed the title Add 'docker run' example Add 'docker run' example / incorrect docker result Feb 20, 2020
@fallion fallion added documentation Improvements or additions to documentation DX DX improvements enhancement New feature or request labels Feb 20, 2020
@fallion
Copy link
Member

fallion commented Feb 20, 2020

First part I will add this week to the README. Thanks again for finding issues like this.

As for:

But the result is incorrect. I ran the above container from a repository with a master branch containing 71 commits, but commitsar just finds the latest commit and reports 1 of 1 commits are not conventional commit compliant. Why are the other 70 commits ignored?

Can you check the discussion here? #167 (comment) We need to find a balance of how far to check on master due to repos not having adopted the conventional commit standard in the past.

@fallion
Copy link
Member

fallion commented Feb 20, 2020

I'm going to add 2 flags which will sort this out. --all which is the use you're looking for and --limit which will limit the number of checked commits.

@tommyalatalo
Copy link
Author

I'm going to add 2 flags which will sort this out. --all which is the use you're looking for and --limit which will limit the number of checked commits.

That would work, but shouldn't "all" be the default behavior? Limiting the range of commits to be checked seems like perfect flag material. What would running commitsar without flags do otherwise?

@fallion
Copy link
Member

fallion commented Feb 20, 2020

Currently it goes like this:
on branch -> All commits are checked until the upstream
on master -> Last commit is checked in order to not block all repositories that were not conventional commit compliant before

There was a suggestion to mirror GIT_DEPTH as well for how far to check on master. It's primarily an issue during adoption. After that it is fine, but if you have very large repositories you do not want to go too far back as it would constantly fail on master in those repositories.

@tommyalatalo
Copy link
Author

tommyalatalo commented Feb 20, 2020

Currently it goes like this:
on branch -> All commits are checked until the upstream
on master -> Last commit is checked in order to not block all repositories that were not conventional commit compliant before

There was a suggestion to mirror GIT_DEPTH as well for how far to check on master. It's primarily an issue during adoption. After that it is fine, but if you have very large repositories you do not want to go too far back as it would constantly fail on master in those repositories.

Either way works I guess, I hope the --all flag can get implemented soon, I'm itching to get this into my pipeline for the master branch!

@fallion
Copy link
Member

fallion commented Feb 20, 2020

It’ll be up tomorrow. :)

@tommyalatalo
Copy link
Author

It’ll be up tomorrow. :)

Fantastic

@fallion fallion mentioned this issue Feb 21, 2020
@fallion
Copy link
Member

fallion commented Feb 24, 2020

@tommyalatalo
Copy link
Author

tommyalatalo commented Feb 24, 2020

Looking good! WIll you push a new image to docker hub?

@fallion
Copy link
Member

fallion commented Feb 24, 2020

@tommyalatalo Pushed. I migrated this repo to GH Actions, but clearly forgot the docker hub push. 🤦‍♂

@tommyalatalo
Copy link
Author

tommyalatalo commented Feb 25, 2020

There is no tag "0.11.0" on docker hub?

@fallion
Copy link
Member

fallion commented Feb 25, 2020

0.11.1 (latest release) is. I’ll release 0.11.0 ASAP.

@tommyalatalo
Copy link
Author

0.11.1 (latest release) is. I’ll release 0.11.0 ASAP.

No its not, at least not tagged as "0.11.1"

docker pull commitsar/commitsar:0.11.1
Error response from daemon: manifest for commitsar/commitsar:0.11.1 not found: manifest unknown: manifest unknown

@fallion
Copy link
Member

fallion commented Feb 25, 2020

I see the issue. https://hub.docker.com/r/outillage/commitsar

There was a rebranding under the outillage name. I’ll set up the backwards compatibility with the old namespace and set up a deprecation strategy.

@fallion
Copy link
Member

fallion commented Feb 25, 2020

I need to improve communication on such issue, however I need to find the correct medium.

@fallion
Copy link
Member

fallion commented Mar 2, 2020

@tommyalatalo It has been returned to the old namespace. Give 0.11.2 a try

@fallion
Copy link
Member

fallion commented Jun 11, 2020

@tommyalatalo is it working ok for you? Can I close this after adding it to docs?

@fallion
Copy link
Member

fallion commented Jun 11, 2020

@tommyalatalo
Copy link
Author

It works, but you shouldn't really need to define the workdir though. If you just write the entrypoint to always run in /src then you can just state in the docs that you always mount the git repo to be checked into /src and you can skip setting the workdir. Would be one thing less to consider.

@fallion
Copy link
Member

fallion commented Jun 12, 2020

Good point, will fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation DX DX improvements enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants