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

align properties better with our existing integrations; Add a few ne… #8

Merged
merged 18 commits into from Aug 15, 2019

Conversation

@gcooney
Copy link
Contributor

commented Aug 7, 2019

Couple main things here:

  1. Remove the checked in compiled typescript and node modules. Add scripts and docs around only checking these on release branches
  2. Standardize property names with azure devops pipelines
  3. Add PR information when available
align properties better with our existing integrations; Add a few new…
… propertiesl retrieve related pull request

@gcooney gcooney added the WIP label Aug 7, 2019

gcooney added 8 commits Aug 8, 2019

@gcooney gcooney force-pushed the snake_cse branch from 0de37b9 to 2b6a469 Aug 14, 2019

@@ -5,7 +5,8 @@
"main": "lib/index.js",
"scripts": {
"test": "tsc && mocha ./tests/_suite.js",
"build": "tsc"
"build": "tsc",
"release": "npm install && npm run build && npm prune --production && git add -f node_modules/* && git add -f lib/*"

This comment has been minimized.

Copy link
@gcooney

gcooney Aug 14, 2019

Author Contributor

script most of release process to simplify

@gcooney gcooney requested review from rsj, simonychoy, gone-skiing and mikedecosta and removed request for rsj Aug 14, 2019

@gcooney gcooney removed the WIP label Aug 14, 2019

gcooney added 2 commits Aug 14, 2019
Revert "test removing github token"
This reverts commit 2dd5e5b.
CONTRIBUTING.md Outdated Show resolved Hide resolved
committer: string | undefined;
repositoryAction: string | undefined;
repositoryBranchName: string | undefined;
repositoryRevisionNumber: string | undefined;

This comment has been minimized.

Copy link
@simonychoy

simonychoy Aug 15, 2019

Was the revision number the commit hash or something? And does it match to one of the properties below?

This comment has been minimized.

Copy link
@gcooney

gcooney Aug 15, 2019

Author Contributor

its mapped to the top level 'revision' instead of being in the properties map. There is a value we support in the properties map as well but I'm nor sure we need to double send it.

repository_pull_request_number?: number | undefined;
repository_pull_request_title?: string | undefined;
repository_pull_request_merged_at?: string | undefined;
repository_pull_request_created_at?: string | undefined;

This comment has been minimized.

Copy link
@simonychoy

simonychoy Aug 15, 2019

Is there a reason not to use the PullRequest type instead?

This comment has been minimized.

Copy link
@gcooney

gcooney Aug 15, 2019

Author Contributor

I think the api properties bundle is currently looking for string -> string.

This comment has been minimized.

Copy link
@gcooney

gcooney Aug 15, 2019

Author Contributor

I initially had it that way but was getting 500s

@simonychoy
Copy link

left a comment

Just a couple of nits.

CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved

@gcooney gcooney merged commit e5dade7 into master Aug 15, 2019

1 check passed

Mabl Test
Details

@gcooney gcooney deleted the snake_cse branch Aug 15, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.