Skip to content
This repository was archived by the owner on Jul 1, 2024. It is now read-only.

Commit 21ce809

Browse files
jablkoelibarzilay
jablko
authored and
elibarzilay
committed
Don't bot-error on invalid DT header
Authors sometimes do [submit invalid DT headers](DefinitelyTyped/DefinitelyTyped#49841), a case which should require author action. This minimal change treats invalid DT headers (including a blank `index.d.ts`) the same way we already treat a missing `index.d.ts`. It only targets freshly invalid DT headers because an existing invalid DT header 1. should never happen 2. probably is a bot error since we can't tell if owners are being added, edited or what. It doesn't matter in the fresh case because the package isn't valid regardless.
1 parent 3a073c5 commit 21ce809

File tree

7 files changed

+317
-7
lines changed

7 files changed

+317
-7
lines changed

‎src/_tests/fixtures/49841/_downloads.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
{}

‎src/_tests/fixtures/49841/_files.json

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"4849e8876b0ab7adc889ccc35b55fde6f0274837:types/react-native-sha1/tsconfig.json": "{\r\n \"compilerOptions\": {\r\n \"module\": \"commonjs\",\r\n \"lib\": [\r\n \"es6\"\r\n ],\r\n \"noImplicitAny\": true,\r\n \"noImplicitThis\": true,\r\n \"strictNullChecks\": true,\r\n \"strictFunctionTypes\": true,\r\n \"baseUrl\": \"../\",\r\n \"typeRoots\": [\r\n \"../\"\r\n ],\r\n \"types\": [],\r\n \"noEmit\": true,\r\n \"target\": \"es6\",\r\n \"forceConsistentCasingInFileNames\": true\r\n },\r\n \"files\": [\r\n \"index.d.ts\",\r\n \"react-native-sha1-test.ts\"\r\n ]\r\n}",
3+
"4849e8876b0ab7adc889ccc35b55fde6f0274837:types/react-native-sha1/tslint.json": "{ \"extends\": \"dtslint/dt.json\" }",
4+
"4849e8876b0ab7adc889ccc35b55fde6f0274837:types/react-native-sha1/index.d.ts": "// Definitions by: Amir Hossein Shekari <https://github.com/shekari-ah>\r\n\r\n\r\n\r\nexport function sha1(input: string): Promise<string>;"
5+
}

‎src/_tests/fixtures/49841/_response.json

Lines changed: 171 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,171 @@
1+
{
2+
"data": {
3+
"repository": {
4+
"pullRequest": {
5+
"id": "MDExOlB1bGxSZXF1ZXN0NTI4OTE2Mjg5",
6+
"title": "Add react-native-sha1 typescript type",
7+
"createdAt": "2020-11-28T09:17:28Z",
8+
"author": {
9+
"login": "shekari-ah",
10+
"__typename": "User"
11+
},
12+
"authorAssociation": "NONE",
13+
"baseRef": {
14+
"name": "master",
15+
"__typename": "Ref"
16+
},
17+
"changedFiles": 4,
18+
"labels": {
19+
"nodes": [
20+
{
21+
"name": "Mergebot Error",
22+
"__typename": "Label"
23+
}
24+
],
25+
"__typename": "LabelConnection"
26+
},
27+
"isDraft": false,
28+
"mergeable": "MERGEABLE",
29+
"number": 49841,
30+
"state": "OPEN",
31+
"headRefOid": "4849e8876b0ab7adc889ccc35b55fde6f0274837",
32+
"timelineItems": {
33+
"nodes": [
34+
{
35+
"__typename": "IssueComment",
36+
"author": {
37+
"login": "typescript-bot",
38+
"__typename": "User"
39+
},
40+
"createdAt": "2020-11-28T09:18:08Z"
41+
}
42+
],
43+
"__typename": "PullRequestTimelineItemsConnection"
44+
},
45+
"reviews": {
46+
"nodes": [],
47+
"__typename": "PullRequestReviewConnection"
48+
},
49+
"commits": {
50+
"totalCount": 1,
51+
"nodes": [
52+
{
53+
"commit": {
54+
"checkSuites": {
55+
"nodes": [
56+
{
57+
"app": {
58+
"name": "GitHub Actions",
59+
"__typename": "App"
60+
},
61+
"conclusion": "FAILURE",
62+
"resourcePath": "/DefinitelyTyped/DefinitelyTyped/commit/4849e8876b0ab7adc889ccc35b55fde6f0274837/checks?check_suite_id=1574877739",
63+
"status": "COMPLETED",
64+
"url": "https://github.com/DefinitelyTyped/DefinitelyTyped/commit/4849e8876b0ab7adc889ccc35b55fde6f0274837/checks?check_suite_id=1574877739",
65+
"__typename": "CheckSuite"
66+
},
67+
{
68+
"app": {
69+
"name": "Azure Pipelines",
70+
"__typename": "App"
71+
},
72+
"conclusion": "FAILURE",
73+
"resourcePath": "/DefinitelyTyped/DefinitelyTyped/commit/4849e8876b0ab7adc889ccc35b55fde6f0274837/checks?check_suite_id=1574878204",
74+
"status": "COMPLETED",
75+
"url": "https://github.com/DefinitelyTyped/DefinitelyTyped/commit/4849e8876b0ab7adc889ccc35b55fde6f0274837/checks?check_suite_id=1574878204",
76+
"__typename": "CheckSuite"
77+
}
78+
],
79+
"__typename": "CheckSuiteConnection"
80+
},
81+
"status": null,
82+
"authoredDate": "2020-11-28T09:12:27Z",
83+
"committedDate": "2020-11-28T09:12:27Z",
84+
"pushedDate": "2020-11-28T09:12:34Z",
85+
"abbreviatedOid": "4849e88",
86+
"oid": "4849e8876b0ab7adc889ccc35b55fde6f0274837",
87+
"__typename": "Commit"
88+
},
89+
"__typename": "PullRequestCommit"
90+
}
91+
],
92+
"__typename": "PullRequestCommitConnection"
93+
},
94+
"comments": {
95+
"totalCount": 1,
96+
"nodes": [
97+
{
98+
"id": "MDEyOklzc3VlQ29tbWVudDczNTE3ODAxNw==",
99+
"author": {
100+
"login": "typescript-bot",
101+
"__typename": "User"
102+
},
103+
"body": "@shekari-ah — There was an error that prevented me from properly processing this PR:\n\n error parsing owners: At 1:1 : Expected /\\/\\/ Type definitions for (non-npm package )?/\n<!--typescript_bot_had-error-->",
104+
"createdAt": "2020-11-28T09:18:08Z",
105+
"reactions": {
106+
"nodes": [],
107+
"__typename": "ReactionConnection"
108+
},
109+
"__typename": "IssueComment"
110+
}
111+
],
112+
"__typename": "IssueCommentConnection"
113+
},
114+
"files": {
115+
"nodes": [
116+
{
117+
"path": "types/react-native-sha1/index.d.ts",
118+
"additions": 5,
119+
"deletions": 0,
120+
"__typename": "PullRequestChangedFile"
121+
},
122+
{
123+
"path": "types/react-native-sha1/react-native-sha1-test.ts",
124+
"additions": 5,
125+
"deletions": 0,
126+
"__typename": "PullRequestChangedFile"
127+
},
128+
{
129+
"path": "types/react-native-sha1/tsconfig.json",
130+
"additions": 24,
131+
"deletions": 0,
132+
"__typename": "PullRequestChangedFile"
133+
},
134+
{
135+
"path": "types/react-native-sha1/tslint.json",
136+
"additions": 1,
137+
"deletions": 0,
138+
"__typename": "PullRequestChangedFile"
139+
}
140+
],
141+
"__typename": "PullRequestChangedFileConnection"
142+
},
143+
"projectCards": {
144+
"nodes": [
145+
{
146+
"id": "MDExOlByb2plY3RDYXJkNTAyMTU4MjU=",
147+
"project": {
148+
"id": "MDc6UHJvamVjdDM3NDExMDQ=",
149+
"number": 5,
150+
"name": "New Pull Request Status Board",
151+
"__typename": "Project"
152+
},
153+
"column": {
154+
"id": "MDEzOlByb2plY3RDb2x1bW43NTUyOTI2",
155+
"name": "Other",
156+
"__typename": "ProjectColumn"
157+
},
158+
"__typename": "ProjectCard"
159+
}
160+
],
161+
"__typename": "ProjectCardConnection"
162+
},
163+
"__typename": "PullRequest"
164+
},
165+
"__typename": "Repository"
166+
}
167+
},
168+
"loading": false,
169+
"networkStatus": 7,
170+
"stale": false
171+
}

‎src/_tests/fixtures/49841/derived.json

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
{
2+
"type": "info",
3+
"now": "2020-11-30T23:30:29.827Z",
4+
"pr_number": 49841,
5+
"author": "shekari-ah",
6+
"headCommitAbbrOid": "4849e88",
7+
"headCommitOid": "4849e8876b0ab7adc889ccc35b55fde6f0274837",
8+
"lastPushDate": "2020-11-28T09:12:34.000Z",
9+
"lastActivityDate": "2020-11-28T09:17:28.000Z",
10+
"maintainerBlessed": false,
11+
"hasMergeConflict": false,
12+
"isFirstContribution": false,
13+
"popularityLevel": "Well-liked by everyone",
14+
"pkgInfo": [
15+
{
16+
"name": "react-native-sha1",
17+
"kind": "add",
18+
"files": [
19+
{
20+
"path": "types/react-native-sha1/index.d.ts",
21+
"kind": "definition"
22+
},
23+
{
24+
"path": "types/react-native-sha1/react-native-sha1-test.ts",
25+
"kind": "test"
26+
},
27+
{
28+
"path": "types/react-native-sha1/tsconfig.json",
29+
"kind": "package-meta",
30+
"suspect": "not the required form"
31+
},
32+
{
33+
"path": "types/react-native-sha1/tslint.json",
34+
"kind": "package-meta-ok"
35+
}
36+
],
37+
"owners": [],
38+
"addedOwners": [],
39+
"deletedOwners": [],
40+
"popularityLevel": "Well-liked by everyone"
41+
}
42+
],
43+
"reviews": [],
44+
"ciResult": "fail",
45+
"ciUrl": "https://github.com/DefinitelyTyped/DefinitelyTyped/commit/4849e8876b0ab7adc889ccc35b55fde6f0274837/checks?check_suite_id=1574877739"
46+
}

‎src/_tests/fixtures/49841/mutations.json

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
[
2+
{
3+
"query": "mutation($input: AddLabelsToLabelableInput!) { addLabelsToLabelable(input: $input) { clientMutationId } }",
4+
"variables": {
5+
"input": {
6+
"labelIds": [
7+
"MDU6TGFiZWwyMDk2NzQzNjAw",
8+
"MDU6TGFiZWw2NDY3ODg4ODg="
9+
],
10+
"labelableId": "MDExOlB1bGxSZXF1ZXN0NTI4OTE2Mjg5"
11+
}
12+
}
13+
},
14+
{
15+
"query": "mutation($input: RemoveLabelsFromLabelableInput!) { removeLabelsFromLabelable(input: $input) { clientMutationId } }",
16+
"variables": {
17+
"input": {
18+
"labelIds": [
19+
"MDU6TGFiZWwyMTk3MzU2OTA1"
20+
],
21+
"labelableId": "MDExOlB1bGxSZXF1ZXN0NTI4OTE2Mjg5"
22+
}
23+
}
24+
},
25+
{
26+
"query": "mutation($input: MoveProjectCardInput!) { moveProjectCard(input: $input) { clientMutationId } }",
27+
"variables": {
28+
"input": {
29+
"cardId": "MDExOlByb2plY3RDYXJkNTAyMTU4MjU=",
30+
"columnId": "MDEzOlByb2plY3RDb2x1bW43NTUyOTI0"
31+
}
32+
}
33+
},
34+
{
35+
"query": "mutation($input: AddCommentInput!) { addComment(input: $input) { clientMutationId } }",
36+
"variables": {
37+
"input": {
38+
"subjectId": "MDExOlB1bGxSZXF1ZXN0NTI4OTE2Mjg5",
39+
"body": "@shekari-ah Thank you for submitting this PR!\n\n***This is a live comment which I will keep updated.***\n\n## 1 package in this PR\n\n* `react-native-sha1` (*new!*) [on npm](https://www.npmjs.com/package/react-native-sha1), [on unpkg](https://unpkg.com/browse/react-native-sha1@latest/)\n\n## Code Reviews\n\nThis PR adds a new definition, so it needs to be reviewed by a DT maintainer before it can be merged.\n\n## Status\n\n * ✅ No merge conflicts\n * ❌ Continuous integration tests have passed\n * ❌ Only a DT maintainer can approve changes when there are new packages added\n\nOnce every item on this list is checked, I'll ask you for permission to merge and publish the changes.\n\n----------------------\n... diagnostics scrubbed ...\n<!--typescript_bot_welcome-->"
40+
}
41+
}
42+
},
43+
{
44+
"query": "mutation($input: AddCommentInput!) { addComment(input: $input) { clientMutationId } }",
45+
"variables": {
46+
"input": {
47+
"subjectId": "MDExOlB1bGxSZXF1ZXN0NTI4OTE2Mjg5",
48+
"body": "🔔 @shekari-ah — you're the only owner, but it would still be good if you find someone to [review this PR](https://github.com/DefinitelyTyped/DefinitelyTyped/pull/49841/files) in the next few days, otherwise a maintainer will look at it. (And if you do find someone, maybe even recruit them to be a second owner to make future changes easier...)\n<!--typescript_bot_pinging-reviewers-others-->"
49+
}
50+
}
51+
},
52+
{
53+
"query": "mutation($input: AddCommentInput!) { addComment(input: $input) { clientMutationId } }",
54+
"variables": {
55+
"input": {
56+
"subjectId": "MDExOlB1bGxSZXF1ZXN0NTI4OTE2Mjg5",
57+
"body": "@shekari-ah The CI build failed! Please [review the logs for more information](https://github.com/DefinitelyTyped/DefinitelyTyped/commit/4849e8876b0ab7adc889ccc35b55fde6f0274837/checks?check_suite_id=1574877739).\r\n\r\nOnce you've pushed the fixes, the build will automatically re-run. Thanks!\n<!--typescript_bot_gh-actions-complaint-4849e88-->"
58+
}
59+
}
60+
}
61+
]

‎src/_tests/fixtures/49841/result.json

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
{
2+
"pr_number": 49841,
3+
"targetColumn": "Needs Author Action",
4+
"labels": [
5+
"The CI failed",
6+
"New Definition"
7+
],
8+
"responseComments": [
9+
{
10+
"tag": "welcome",
11+
"status": "@shekari-ah Thank you for submitting this PR!\n\n***This is a live comment which I will keep updated.***\n\n## 1 package in this PR\n\n* `react-native-sha1` (*new!*) [on npm](https://www.npmjs.com/package/react-native-sha1), [on unpkg](https://unpkg.com/browse/react-native-sha1@latest/)\n\n## Code Reviews\n\nThis PR adds a new definition, so it needs to be reviewed by a DT maintainer before it can be merged.\n\n## Status\n\n * ✅ No merge conflicts\n * ❌ Continuous integration tests have passed\n * ❌ Only a DT maintainer can approve changes when there are new packages added\n\nOnce every item on this list is checked, I'll ask you for permission to merge and publish the changes.\n\n----------------------\n... diagnostics scrubbed ..."
12+
},
13+
{
14+
"tag": "pinging-reviewers-others",
15+
"status": "🔔 @shekari-ah — you're the only owner, but it would still be good if you find someone to [review this PR](https://github.com/DefinitelyTyped/DefinitelyTyped/pull/49841/files) in the next few days, otherwise a maintainer will look at it. (And if you do find someone, maybe even recruit them to be a second owner to make future changes easier...)"
16+
},
17+
{
18+
"tag": "gh-actions-complaint-4849e88",
19+
"status": "@shekari-ah The CI build failed! Please [review the logs for more information](https://github.com/DefinitelyTyped/DefinitelyTyped/commit/4849e8876b0ab7adc889ccc35b55fde6f0274837/checks?check_suite_id=1574877739).\r\n\r\nOnce you've pushed the fixes, the build will automatically re-run. Thanks!"
20+
}
21+
],
22+
"shouldClose": false,
23+
"shouldMerge": false,
24+
"shouldUpdateLabels": true,
25+
"shouldUpdateProjectColumn": true,
26+
"shouldRemoveFromActiveColumns": false,
27+
"isReadyForAutoMerge": false
28+
}

‎src/pr-info.ts

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -332,15 +332,13 @@ async function getPackageInfosEtc(
332332
: !paths.includes(`types/${name}/index.d.ts`) ? oldOwners
333333
: await getOwnersOfPackage(name, headId, fetchFile);
334334
if (oldOwners instanceof Error) return oldOwners;
335-
if (newOwners instanceof Error) return newOwners;
336-
if (name && !oldOwners && !newOwners) return new Error("could not get either old or new owners");
337-
const kind = !name ? "edit" : oldOwners && newOwners ? "edit" : newOwners ? "add" : "delete";
335+
const kind = !name ? "edit" : oldOwners ? newOwners ? "edit" : "delete" : "add";
338336
const owners = oldOwners || [];
339-
const addedOwners = oldOwners === null ? (newOwners || [])
340-
: newOwners === null ? []
337+
const addedOwners = newOwners === null || newOwners instanceof Error ? []
338+
: oldOwners === null ? newOwners
341339
: newOwners.filter(o => !oldOwners.includes(o));
342-
const deletedOwners = newOwners === null ? owners
343-
: oldOwners === null ? []
340+
const deletedOwners = oldOwners === null ? []
341+
: newOwners === null || newOwners instanceof Error ? oldOwners
344342
: oldOwners.filter(o => !newOwners.includes(o));
345343
// null name => infra => ensure critical (even though it's unused atm)
346344
const downloads = name ? await getDownloads(name) : Infinity;

0 commit comments

Comments
 (0)