danger / danger-js

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

Gitlab rRest api doesn't accept Project name anymore #969

Open farabi opened 4 years ago

farabi commented 4 years ago

Hello.

Danger is trying to call the Gitlab rest api using the project name. while gitlab now only accepts the project ID as you can see in the documentation.

Here is the error, i get.

nesterenkodm commented 4 years ago

same

orta commented 4 years ago

Cool, makes sense - someone want to take a look at a fix?

farabi commented 4 years ago

@orta I will !

farabi commented 4 years ago

@orta
Dumb question, but how to compile a danger-js to use it as a danger-swift dependency ?

HelloCore commented 4 years ago

@orta Dumb question, but how to compile a danger-js to use it as a danger-swift dependency ?

I believe it's this command yarn run build; yarn run pkg . --output brew-distribution/danger;. Copied from https://github.com/danger/danger-js/blob/dab0e63a97705617c0e0017d85a9ff7129cadd5f/package.json#L69

f-meloni commented 4 years ago

that could not work given https://github.com/danger/danger-js/issues/958 :(

HelloCore commented 4 years ago

@farabi The pr https://github.com/danger/danger-js/pull/970 got merged to master, you could use yarn run build; yarn run pkg . --output brew-distribution/danger; to build the binary. Also, don’t forget to run yarn install

farabi commented 4 years ago

Hello @orta @chebur

Actually gitlab does accept project name, the issue is coming from apache configuration where local gitlab is hosted. Unless AllowEncodedSlashes is set to on. rest calls with encoded slashes will not work. Like in my case : "https://gitlab.xxxx.com/api/v4/projects/project%2Fname/merge_requests/15/changes"

The solution i did in my fork is to add a env variable "DANGER_GITLAB_PROJECT_ID". and if it's not defined then we use the project name.

Lot of tools are using the same approach with the project id.

If its ok for you, i add a pull request about that. otherwise , we close the ticket and i will keep using my fork. farabi/tap/danger-swift

orta commented 4 years ago

Seems like a good PR to me 👍

lazy-var commented 4 years ago

In addition to @farabi apache configuration suggestion, I needed NoDecode and nocanon parameters:

<VirtualHost *:port>
  AllowEncodedSlashes NoDecode

  <Location /example/>
    ProxyPass http://anotherserver:8080/example nocanon
  </Location>
</VirtualHost>

as explained here: https://stackoverflow.com/a/9933890

AnthonyMastrean commented 4 years ago

Edit: Turns out I was observing a different API URL construction issue... https://github.com/danger/danger-js/issues/1028

ivankatliarchuk commented 2 years ago

The intention is right to support project number ID, but Gitlab does support The ID or [URL-encoded path of the project](https://docs.gitlab.com/ee/api/index.html#namespaced-path-encoding). so both are working.