-
Notifications
You must be signed in to change notification settings - Fork 0
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
feat(bitbucket): add support for constructing pull_request
payloads
#23
Conversation
This is a more reliable place to look in the github payloads for the organization slug. Fixes: #22
pull_request
payloads
// https://git-scm.com/docs/pretty-formats#Documentation/pretty-formats.txt-emctem | ||
// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Date/toISOString | ||
timestamp: new Date( | ||
execSync('git show -s --format=%ct $BITBUCKET_COMMIT', { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should these be in UTC? if so: git show … --date=unix
works
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the timestamp for the head commit, not the last commit (ref, is what Micro should store?
git rev-parse origin/$BITBUCKET_PR_DESTINATION_BRANCH
...baseContext, | ||
eventName, | ||
payload: { | ||
number: process.env.BITBUCKET_PR_ID, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how about parseInt
'ing this - to keep the payload closer resembling to GH's?
the GH fixture pull_request.opened.json
shows number: 2
. it's a number in GH payloads, and a number in the Micro DB schema.
I wasn't entirely clear from PR #373 of how it's getting coerced, since I didn't see any changes to the pr
object where number
is assigned to data.payload.number
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah that's not a bad idea! I actually modified the json schema stuff in the backend to coerce types now, so it matters less. But probably a good idea to be as consistent as possible.
name: process.env.BITBUCKET_REPO_SLUG, | ||
}, | ||
organization: { | ||
login: process.env.BITBUCKET_WORKSPACE, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in the near future: should we be storing the workspace UUIDs and repo UUIDs for BB (and IDs for GH)?
name: ReadMe Micro | ||
image: node:18 | ||
script: | ||
- npx @readme/[email protected] '**/*.{yaml,yml,json}' --key=$README_MICRO_SECRET |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could use conditions to run Micro only for this glob, but that'll add complexity when folks are integrating Micro into their bitbucket-pipelines.yml
.
in an ideal world, this repo would have a Dockerfile, a pipe.yml
, etc. so Bitbucket could use that Pipe, instead this getting converted to its own docker image before the Pipeline is run.
the number of lines needed for folks to plug in the Micro pipe in to their bitbucket-pipelines.yml
file, but that can probably wait until we see Bitbucket customers requesting it.
🧰 Changes
Add support for constructing
pull_request
payloadsTODO
🧬 QA & Testing
Do the tests pass? More thorough instructions incoming!