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

Additional refactoring #234

Merged
merged 1 commit into from
Nov 24, 2020
Merged

Additional refactoring #234

merged 1 commit into from
Nov 24, 2020

Conversation

elibarzilay
Copy link
Contributor

  • Switch Action.labels from an object to a list of labels

    Since there are now many labels, this makes the test files smaller,
    and also the labels are much easier to read. Also, the code is a bit
    simpler (modulo the as const thing).

    • Very minor caveat: the order of labels in the result is sometimes
      different now.

    • Very minor bonus: test 44402 has the only difference, which is due
      to a very minor fix. Previously, if the labels were {X: true},
      then the execute-pr-actions code would only touch that. There's a
      bit in compute-pr-actions that returns labels: ["Edits Infrastructure"], which previously made it add that label but leave
      all others, whereas now other (known) labels are removed.

  • In preparation for the timeline change:

    • Replace stalenessInDays with lastActivityDate (so staleness is
      computed later).

    • Remove unused reopenedDate, lastCommentDate.

  • Also:

    • Switch from the ApprovalFlags bitmap to a ApproverKind[] with a
      matching classification of each review.

    • Add needsAuthorAction.

    • Add a convenient post function to make comment-posting code more
      readable.

    • Add a :passport_control: to the reply for premature merge
      requests.

    • Reindent and refactor execute-pr-actions to make it more functional.

    • Slightly clearer definitions for earliestDate, latestDate.

    • Switch import * as Comment -> comment.

* Switch `Action.labels` from an object to a list of labels

  Since there are now many labels, this makes the test files smaller,
  and also the labels are much easier to read.  Also, the code is a bit
  simpler (modulo the `as const` thing).

  - Very minor caveat: the order of labels in the result is sometimes
    different now.

  - Very minor bonus: test 44402 has the only difference, which is due
    to a very minor fix.  Previously, if the labels were `{X: true}`,
    then the `execute-pr-actions` code would only touch that.  There's a
    bit in `compute-pr-actions` that returns `labels: ["Edits
    Infrastructure"]`, which previously made it add that label but leave
    all others, whereas now other (known) labels are removed.

* In preparation for the timeline change:

  - Replace `stalenessInDays` with `lastActivityDate` (so staleness is
    computed later).

  - Remove unused `reopenedDate`, `lastCommentDate`.

* Also:

  - Switch from the `ApprovalFlags` bitmap to a `ApproverKind[]` with a
    matching classification of each review.

  - Add `needsAuthorAction`.

  - Add a convenient `post` function to make comment-posting code more
    readable.

  - Add a `:passport_control:` to the reply for premature merge
    requests.

  - Reindent and refactor `execute-pr-actions` to make it more functional.

  - Slightly clearer definitions for `earliestDate`, `latestDate`.

  - Switch `import * as Comment` -> `comment`.
"lastPushDate": "2020-01-03T21:02:41.000Z",
"reopenedDate": "2019-10-18T22:52:06.000Z",
"lastCommentDate": "2020-05-12T15:13:19.000Z",
"lastActivityDate": "2020-05-12T15:13:19.000Z",
"maintainerBlessed": false,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great

@@ -51,7 +51,7 @@
"variables": {
"input": {
"subjectId": "MDExOlB1bGxSZXF1ZXN0NTAwNjg4ODM0",
"body": "Hi @mgol, I can't [accept a merge request](https://github.com/DefinitelyTyped/DefinitelyTyped#make-a-pull-request) until the PR has a green CI and was appropriately reviewed. I will let you know once that happens.\n\nThanks, and happy typing!\n<!--typescript_bot_wait-for-merge-offer-06d8d67-->"
"body": ":passport_control: Hi @mgol,\n\nI can't [accept a merge request](https://github.com/DefinitelyTyped/DefinitelyTyped#make-a-pull-request) until the PR has a green CI and was appropriately reviewed. I will let you know once that happens.\n\nThanks, and happy typing!\n<!--typescript_bot_wait-for-merge-offer-06d8d67-->"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cute

return who === "maintainers" && blessed && !noOtherOwners ? "owners"
: who === "owners" && noOtherOwners ? "maintainers"
return who === "maintainer" && blessed && !noOtherOwners ? "owner"
: who === "owner" && noOtherOwners ? "maintainer"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call on the plural -> singular, feels better

@orta
Copy link
Contributor

orta commented Nov 24, 2020

Nice wrk 👍

@orta orta merged commit e2678f4 into DefinitelyTyped:master Nov 24, 2020
@elibarzilay elibarzilay deleted the more-revisions branch November 25, 2020 05:34
@elibarzilay
Copy link
Contributor Author

Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants