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

Combine dependent actions#348

Merged
elibarzilay merged 1 commit intoDefinitelyTyped:masterfrom
jablko:patch-80
Feb 1, 2021
Merged

Combine dependent actions#348
elibarzilay merged 1 commit intoDefinitelyTyped:masterfrom
jablko:patch-80

Conversation

@jablko
Copy link
Copy Markdown
Contributor

@jablko jablko commented Jan 21, 2021

Minor detail but it could be more obvious, when we set targetColumn, shouldUpdateProjectColumn and shouldRemoveFromActiveColumns independently, what the result will be. What do you think about combining these three and replacing shouldRemoveFromActiveColumns with "REMOVE"?

Also I replaced "context" with "actions" in compute-pr-actions.ts, to match execute-pr-actions.ts, etc.

@jablko jablko force-pushed the patch-80 branch 2 times, most recently from 9451735 to 2d870a5 Compare January 21, 2021 18:56
@jablko jablko changed the title Combine targetColumn, shouldUpdateProjectColumn and shouldRemoveFromActiveColumns Combine dependent actions Jan 21, 2021
Instead of having independent `targetColumn`,
`shouldUpdateProjectColumn`, and `shouldRemoveFromActiveColumns`.

Update snapshots.
Copy link
Copy Markdown
Contributor

@elibarzilay elibarzilay left a comment

Choose a reason for hiding this comment

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

LGTM, except that it slightly bugs me to use REMOVE, so I tweaked it to *REMOVE* instead.

@elibarzilay elibarzilay merged commit f4a8020 into DefinitelyTyped:master Feb 1, 2021
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