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

Problems with Danger #369

Closed
PascalTurbo opened this issue Jan 24, 2017 · 6 comments
Closed

Problems with Danger #369

PascalTurbo opened this issue Jan 24, 2017 · 6 comments

Comments

@PascalTurbo
Copy link

Hi There,

how is it possible, that there are commits in our Project that doesn't fit Dangers rules.
For example 4716fa7 has a title which is to long.

The other question would be: Does it make sense to use a tool which the developer can't execute locally to check if the Pull Request fits the rules?

@HParker
Copy link
Member

HParker commented Jan 24, 2017

@jonallured
Copy link
Contributor

Hi @PascalTurbo, I'm sorry you're having problems with Danger - it's meant to help automate pull request reviews and reduce the repetitive feedback maintainers sometimes have to give.

I'm not sure I understand your initial question - are you asking how a commit would slip past Danger? If so, Danger only reviews the commits in the PR in question - not all commits in the project. There was also a span of time when it wasn't working well for Feedjira.

Your other question is one I struggle with too - it almost seems unfair to have something that might fail a PR that a dev can't or might not know how to run locally (it's possible, but I've found it awkward). But it's ok for a PR to fail and then for the author to fix - this would happen if they failed the test suite!

Then specifically as it relates to my Danger plugin, Commit Lint: I've been working on improving it's ability to help guide people to fixing their PRs. For instance, with the newest version of the plugin it will encourage one to amend commit messages and force push on their branch to update the PR.

I should PR an update to that plugin and see if that helps!

But I'm open to other ideas too - if you think there are ways that Danger could help this project or projects in general, please let me know!! ❤️

@jonallured
Copy link
Contributor

Oops, forgot we don't check in the Gemfile.lock which means the newest Commit Lint has been in use already. 😄

@phoet
Copy link
Contributor

phoet commented Mar 5, 2017

i can understand that you want to reduce the amount of work for a maintainer, but keep in mind, that if you are greeted with a broken build like here, you really do not motivate me to continue any contributions.

@mikeastock
Copy link
Member

mikeastock commented Mar 6, 2017

@HParker how would we feel about change Danger to warn about commits that don't pass the commit lint but not failing since we can fix the commits on merge?

See #375

@phoet
Copy link
Contributor

phoet commented Mar 6, 2017

that sounds like a good alternative to me. especially since the repo does not have contribution guidelines (or i overlooked them). that would more gently nudge people into it. especially since squash merging makes the whole problem go away if you want to

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

No branches or pull requests

5 participants