Open mrchief opened 5 years ago
Should be good to go now. I copied existing logic into createFinalReport
but I'd appreciate if you can verify those core pieces (lint + createFinalReport). If we can verify those 2 are doing what they are doing in current prod implementation, then this PR should be good to merge.
@mrchief I probably won't get to spend anytime on this today. But I'll outline steps in case you want to get a jumpstart on testing it:
@ewolfe Sounds like a plan. Will keep you posted.
Tried creating a now deployment but things weren't going well. After banging my head in frustration, I found this: https://github.com/probot/probot.github.io/issues/273
now doesn't let you create v1 deployments anymore. I created a v2 now.json
{
"name": "prlint",
"version": 2,
"alias": "prlint-staging",
"env": {
"PRIVATE_KEY": "@prlint-private-key",
"WEBHOOK_SECRET": "@prlint-webhook-secret",
"NODE_ENV": "production"
},
"builds": [
{
"src": "*.js",
"use": "@now/node"
}
],
"routes": [
{
"src": "/", "dest": "index.js"
}
],
"regions": [
"all"
]
}
but it seems like we need that wrapper for things to work properly. Looks like Probot docs would need an update too. Will continue looking into this...
Oh no! That sounds like it was probably really frustrating. I can help investigate over the weekend too.
You bet! 3 hours of head-banging fun! I also tried the serverless-lambda but that resulted in Error: Cannot find module '/var/task/user/package'
error. I'm going to give it a spin in a real AWS lambda (and not via now) to see if it makes any difference.
I tried with AWS lambda and here's what I found:
handler.js
logic differs between AWS and GCF. handler.js
and rest of the package, I was able to get the lambda to respond to install/ping etc events. It didn't mount to /webhook
path even with the Probot env var so either I'm missing something or something else is amiss from Probot's side (could be a bug on them). It's not a big issue for me as /webhook
is something we want to preserve for the now.sh
deployments and not for internal/private deployments (although it'd be nice to have consistency).So based on these, I think we can do one of these:
now.sh
, AWS, GCF specific handler.js
files - not sure if this is a trend or strategy that we should own or let it be handled via userland (I prefer userland since every enterprise will have its own requirements which may not align with what we package or provide). now.sh
specific implementation to its own repo. This'll be practically dogfooding and I think will serve a few different purposes:
Thoughts?
@ewolfe Did you get a chance to look at it yet?
@ewolfe Any update on this? I can merge this but I don't have access to the deployment service (now.sh I believe?).
If you can add me there, then I can test and get this version uploaded.
@mrchief sorry for the delay. Yeah let's get this wrapped up! Can you signup with this URL https://zeit.co/teams/invite/qDB2l5r6 and then try deploying to https://prlint-staging.now.sh/ ?
Great! I signed up but I wasn't able to install Now on prlint (complains that I'm not an owner which is weird). I'll try later via the cli.
I'll ping you on spectrum if needed. Don't want to bog down this PR thread with side chatter.
@mrchief can you try installing it on https://github.com/prlint/prlint-staging ? I want to move this repo over to https://github.com/prlint/prlint as part of this whole update as well
@mrchief can you try installing it on prlint/prlint-staging
I believe that is where I tried. But I'll check again tonight.
Was able to sort thru issues. Did a test deployment at https://prlint-staging.mrchief.now.sh/ so I know integration etc. is now all setup correctly.
I'm going to start cleaning up this PR now.
Fixes #85