danger / danger-js

⚠️ Stop saying "you forgot to …" in code review
http://danger.systems/js/
MIT License
5.24k stars 367 forks source link

Danger 3 Warning Message (why?) #470

Open GantMan opened 6 years ago

GantMan commented 6 years ago

run yarn danger ci warning comes out telling me I didn't run it right.

image

Travis report: https://travis-ci.org/infinitered/solidarity/jobs/324899875

Not sure why Travis got this message, but it's off.

hawkrives commented 6 years ago

See also https://github.com/danger/danger-js/pull/465#issuecomment-355191964; I've got a hunch that the condition for printing that message is flipped?

hawkrives commented 6 years ago

https://github.com/danger/danger-js/blob/aabb0069e5b3cf91d11fdc7b9dd38225ffb95c39/source/commands/danger.ts#L35-L39

So… the condition's not flipped, but for some reason it's still being printed even when invoked as danger ci?

orta commented 6 years ago

It likely needs a check if process.args does not include the strings: init, ci, process, pr or --help

orta commented 6 years ago

I've not had much time for Danger during the work week, so will take a look over the weekend

fbartho commented 6 years ago

I'm getting this error, even though I'm running danger 3.0.3?

$ /Users/greenhouse/tmpXwQA6s/node_modules/.bin/danger ci
You may have updated from Danger 2.x -> 3.x without updating from `danger` to `danger ci`.
Skipping Danger due to this run not executing on a PR.
Done in 1.08s.

Invoked via a shell script that calls yarn danger ci

Note: I'm using the new Nevercode.io CISource, so I don't know if that's related to the issue.

orta commented 6 years ago

this is fixed in https://github.com/danger/danger-js/pull/473

but it isn't shipped yet

fbartho commented 6 years ago

Ahhhh @orta, thanks, I missed that that PR existed and hadn't shipped yet, and my previously working danger setup was on a different CI provider running Danger 2.x.

fbartho commented 6 years ago

@orta --

  1. Is there any way that we can help with your work in #473? (The TODO list looks slightly less clear than I need to jump right in?)
  2. Alternately, can we back-port the fix for this issue back into mainline?

This issue is blocking me from running danger right now as my old CI is gone & I'm leaning towards using Nevercode, but the lack of danger is one of my final concerns on selecting that CI provider.

orta commented 6 years ago

I'm not sure how this is blocking you? This is just a console.error:

https://github.com/danger/danger-js/blob/35fecbc687976fc2702744f3ef5984ad71f17c91/source/commands/danger.ts#L42-L44

It has no effect on behavior, which is why it's not been a priority. Your changes to nevercode were shipped 6 days ago in 3.0.3 - https://github.com/danger/danger-js/pull/474#issuecomment-356294695

fbartho commented 6 years ago

Awesome, then I have a different problem. I thought that was a failure of danger where it was aborting. I didn't look further. -- For some reason danger is doing nothing (I guess silently) when run on Nevercode.

They just shipped some new environment variables this morning -- to make Nevercode more normal as compared to other CI sources for danger's purposes, so I'll patch the cisource to use shortly and see if that fixes things.

orta commented 6 years ago

Yep, try run your command with DEBUG="*" - DEBUG="*" yarn danger ci