From a7301f75c952e9b684c77ab64f5ec37b3b6e61f6 Mon Sep 17 00:00:00 2001 From: jordycosta Date: Wed, 8 Mar 2023 09:25:05 -0700 Subject: [PATCH 01/22] add mergeable field This adds the 'mergeable' field for the pull request The value of the mergeable attribute can be true, false, or null. If the value is null, then GitHub has started a background job to compute the mergeability. Read more: https://docs.github.com/en/rest/pulls/pulls?apiVersion=2022-11-28#get-a-pull-request --- frontend/src/types.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/frontend/src/types.ts b/frontend/src/types.ts index feef6a83..8ea56586 100644 --- a/frontend/src/types.ts +++ b/frontend/src/types.ts @@ -86,6 +86,7 @@ export class PullData { created_at: DateString; updated_at: DateString; closed_at: DateString | null; + mergeable: boolean | null; merged_at: DateString | null; difficulty: number | null; milestone: { From 3cd86830ebfe6d7c6eeaca0c7cd722c4d1d71b3a Mon Sep 17 00:00:00 2001 From: jordycosta Date: Wed, 8 Mar 2023 09:29:10 -0700 Subject: [PATCH 02/22] create function for mergeable state This creates hasMergeConflicts(), which will check if the mergeable state on a pull request is false (i.e. there are some conflicts with merging). --- frontend/src/pull.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/frontend/src/pull.ts b/frontend/src/pull.ts index e1634952..175f82d6 100644 --- a/frontend/src/pull.ts +++ b/frontend/src/pull.ts @@ -140,6 +140,10 @@ export class Pull extends PullData { ); } + hasMergeConflicts(): boolean | null { + return this.mergeable == false; + } + buildStatusesWithRequired(): CommitStatus[] { const statuses = this.buildStatuses(); (this.repoSpec?.requiredStatuses || []).forEach((requiredContext) => { From 57ac44a1c0be0d68350e0ef55b1fc489914a24ef Mon Sep 17 00:00:00 2001 From: jordycosta Date: Wed, 8 Mar 2023 09:31:15 -0700 Subject: [PATCH 03/22] create new flag This creates a new flag on Pulldasher for the a pull request with merge conflicts, the state is reliant on the pull request hasMergeConflicts() function --- frontend/src/pull-card/flags.tsx | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/frontend/src/pull-card/flags.tsx b/frontend/src/pull-card/flags.tsx index 3d09a90d..58c5014d 100644 --- a/frontend/src/pull-card/flags.tsx +++ b/frontend/src/pull-card/flags.tsx @@ -11,6 +11,7 @@ import { faEyeSlash, faSnowflake, faSpinner, + faCodeCompare, } from "@fortawesome/free-solid-svg-icons"; export const Flags = memo(function Flags({ pull }: { pull: Pull }) { @@ -21,6 +22,7 @@ export const Flags = memo(function Flags({ pull }: { pull: Pull }) { const QAing = pull.getLabel("QAing"); const externalBlock = pull.getLabel("external_block"); const cryogenicStorage = pull.getLabel("Cryogenic Storage"); + const mergeConflict = pull.hasMergeConflicts(); return ( <> {deployBlock && ( @@ -86,6 +88,13 @@ export const Flags = memo(function Flags({ pull }: { pull: Pull }) { icon={faSnowflake} /> )} + {mergeConflict && ( + + )} ); }); From c24f35a98a4b7916f996fcf7cce335d2dfb023c3 Mon Sep 17 00:00:00 2001 From: jordycosta Date: Wed, 8 Mar 2023 09:32:24 -0700 Subject: [PATCH 04/22] add sorting for QA column This adds sorting for the pull requests that have merge conflicts or not Sort behavior: Pulls with no merge conflicts go above those with merge conflicts --- frontend/src/pulldasher/sort.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/frontend/src/pulldasher/sort.ts b/frontend/src/pulldasher/sort.ts index b91b4c67..cea2afb4 100644 --- a/frontend/src/pulldasher/sort.ts +++ b/frontend/src/pulldasher/sort.ts @@ -17,6 +17,8 @@ export function QACompare(a: Pull, b: Pull): number { return ( // Pulls I'm QAing above those I'm not compareBool(isQAingByMe(a), isQAingByMe(b)) || + // Pulls with no merge conflicts above those with merge conflicts + compareBool(!a.hasMergeConflicts(), !b.hasMergeConflicts) || // Pulls with no external_block above those with external_block compareBool(!a.getLabel("external_block"), !b.getLabel("external_block")) || // Pulls with no QAing label above those with QAing From 48ee964d505dcde2da036195261271a4d1f6aadd Mon Sep 17 00:00:00 2001 From: jordycosta Date: Wed, 8 Mar 2023 09:33:46 -0700 Subject: [PATCH 05/22] add mergeConflict variant This adds the mergeConflict variant to be used to style the relative flag on Pulldasher This commit also includes the related CSS changes inside of day_theme and night_theme --- frontend/src/theme.tsx | 5 +++++ frontend/src/theme/day_theme.less | 4 ++++ frontend/src/theme/night_theme.less | 4 ++++ 3 files changed, 13 insertions(+) diff --git a/frontend/src/theme.tsx b/frontend/src/theme.tsx index 08919c0c..979fbd69 100644 --- a/frontend/src/theme.tsx +++ b/frontend/src/theme.tsx @@ -210,6 +210,11 @@ export const theme = extendTheme({ backgroundColor: "var(--tag-cryo-background)", borderColor: "var(--tag-cryo-border)", }, + mergeConflict: { + color: "var(--tag-merge-conflict)", + backgroundColor: "var(--tag-merge-conflict-background)", + borderColor: "var(--tag-merge-conflict-border)", + }, }, }, }, diff --git a/frontend/src/theme/day_theme.less b/frontend/src/theme/day_theme.less index bca34fd7..704ac184 100644 --- a/frontend/src/theme/day_theme.less +++ b/frontend/src/theme/day_theme.less @@ -133,6 +133,10 @@ body[data-theme="day_theme"] { --tag-cryo-border: var(--tag-default-border); --tag-cryo-background: @blue; + --tag-merge-conflict: @yellow; + --tag-merge-conflict-border: var(--tag-default-border); + --tag-merge-conflict-background: var(--tag-default-background); + --user-icon: @blue; // Pull age diff --git a/frontend/src/theme/night_theme.less b/frontend/src/theme/night_theme.less index 5af663b2..a30cf87e 100644 --- a/frontend/src/theme/night_theme.less +++ b/frontend/src/theme/night_theme.less @@ -129,6 +129,10 @@ body[data-theme="night_theme"] { --tag-externally-blocked-border: var(--tag-default-border); --tag-externally-blocked-background: var(--tag-default-background); + --tag-merge-conflict: @yellow; + --tag-merge-conflict-border: var(--tag-default-border); + --tag-merge-conflict-background: var(--tag-default-background); + --user-icon: @blue; // Pull age From f746b5b9f1f94a54ba9938c3cd69802c17bc1c8b Mon Sep 17 00:00:00 2001 From: jordycosta Date: Wed, 8 Mar 2023 09:52:01 -0700 Subject: [PATCH 06/22] update return value + sort logic This updates the hasMergeConflicts() return value from boolean | null to just boolean since we are only comparing if this.mergeable is false (which will only return true/false) This also updates the sorting logic, which was broken in a previous commit --- frontend/src/pull.ts | 2 +- frontend/src/pulldasher/sort.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/frontend/src/pull.ts b/frontend/src/pull.ts index 175f82d6..7a5c4778 100644 --- a/frontend/src/pull.ts +++ b/frontend/src/pull.ts @@ -140,7 +140,7 @@ export class Pull extends PullData { ); } - hasMergeConflicts(): boolean | null { + hasMergeConflicts(): boolean { return this.mergeable == false; } diff --git a/frontend/src/pulldasher/sort.ts b/frontend/src/pulldasher/sort.ts index cea2afb4..713b804c 100644 --- a/frontend/src/pulldasher/sort.ts +++ b/frontend/src/pulldasher/sort.ts @@ -18,7 +18,7 @@ export function QACompare(a: Pull, b: Pull): number { // Pulls I'm QAing above those I'm not compareBool(isQAingByMe(a), isQAingByMe(b)) || // Pulls with no merge conflicts above those with merge conflicts - compareBool(!a.hasMergeConflicts(), !b.hasMergeConflicts) || + compareBool(a.hasMergeConflicts(), b.hasMergeConflicts()) || // Pulls with no external_block above those with external_block compareBool(!a.getLabel("external_block"), !b.getLabel("external_block")) || // Pulls with no QAing label above those with QAing From d6b0bbf5e83bc6909cb5f9358a1b8bf0a65c0ff7 Mon Sep 17 00:00:00 2001 From: jordycosta Date: Wed, 8 Mar 2023 11:40:45 -0700 Subject: [PATCH 07/22] send to deploy blocked column if conflicts exist This filters the pull requests with CR/QA fulfilled that have merge conflicts into the Deploy Blocked column in Pulldasher (not considered 'Ready') --- frontend/src/pulldasher/index.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/src/pulldasher/index.tsx b/frontend/src/pulldasher/index.tsx index db3e533d..3fd359c1 100644 --- a/frontend/src/pulldasher/index.tsx +++ b/frontend/src/pulldasher/index.tsx @@ -33,7 +33,7 @@ function Pulldasher() { const allPulls = useAllPulls(); const pulls = useAllOpenPulls(); const pullsCIBlocked = pulls.filter((pull) => pull.isCiBlocked()); - const pullsDeployBlocked = pulls.filter((pull) => pull.isDeployBlocked()); + const pullsDeployBlocked = pulls.filter((pull) => pull.isDeployBlocked() || pull.hasMergeConflicts); const pullsReady = pulls.filter( (pull) => pull.isReady() && pull.isCiRequired() ); From 3e27a02dce80cac3b618d2ce11aa1771dce7d432 Mon Sep 17 00:00:00 2001 From: jordycosta Date: Wed, 8 Mar 2023 11:44:15 -0700 Subject: [PATCH 08/22] update pullsReady logic This updates the logic for a pull request to be considered "Ready" by ensuring that the pull request has no merge conflicts --- frontend/src/pulldasher/index.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/src/pulldasher/index.tsx b/frontend/src/pulldasher/index.tsx index 3fd359c1..4c0c8a53 100644 --- a/frontend/src/pulldasher/index.tsx +++ b/frontend/src/pulldasher/index.tsx @@ -35,7 +35,7 @@ function Pulldasher() { const pullsCIBlocked = pulls.filter((pull) => pull.isCiBlocked()); const pullsDeployBlocked = pulls.filter((pull) => pull.isDeployBlocked() || pull.hasMergeConflicts); const pullsReady = pulls.filter( - (pull) => pull.isReady() && pull.isCiRequired() + (pull) => pull.isReady() && pull.isCiRequired() && !pull.hasMergeConflicts() ); const pullsDevBlocked = pulls.filter( (pull) => pull.getDevBlock() || pull.isDraft() From 4c1b6c11259619b341852f2af5fb46b7e38e1bbd Mon Sep 17 00:00:00 2001 From: jordycosta Date: Wed, 8 Mar 2023 11:45:31 -0700 Subject: [PATCH 09/22] QA Column: Update sorting This updates the sorting on the QA column to ensure that pull requests without merge conflicts show before pull requests with merge conflicts The uses the logical NOT operator on the hasMergeConflicts() function --- frontend/src/pulldasher/sort.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frontend/src/pulldasher/sort.ts b/frontend/src/pulldasher/sort.ts index 713b804c..eb405de4 100644 --- a/frontend/src/pulldasher/sort.ts +++ b/frontend/src/pulldasher/sort.ts @@ -17,10 +17,10 @@ export function QACompare(a: Pull, b: Pull): number { return ( // Pulls I'm QAing above those I'm not compareBool(isQAingByMe(a), isQAingByMe(b)) || - // Pulls with no merge conflicts above those with merge conflicts - compareBool(a.hasMergeConflicts(), b.hasMergeConflicts()) || // Pulls with no external_block above those with external_block compareBool(!a.getLabel("external_block"), !b.getLabel("external_block")) || + // Pulls with no merge conflicts above those with merge conflicts + compareBool(!a.hasMergeConflicts(), !b.hasMergeConflicts()) || // Pulls with no QAing label above those with QAing compareBool(!a.getLabel("QAing"), !b.getLabel("QAing")) || // Pulls with CR completed above those that need more From 5722ec5608f75ad1801d9fc03d896e25f90d247c Mon Sep 17 00:00:00 2001 From: jordycosta Date: Wed, 8 Mar 2023 14:46:43 -0700 Subject: [PATCH 10/22] update DB + schema, append to API call This includes the 'mergeable' field in the GitHub API call This also adds in the 'mergeable' field to get from the DB --- migrations/schema.sql | 1 + models/db_pull.js | 1 + models/pull.js | 2 ++ 3 files changed, 4 insertions(+) diff --git a/migrations/schema.sql b/migrations/schema.sql index dd59ba52..0972e13b 100644 --- a/migrations/schema.sql +++ b/migrations/schema.sql @@ -143,6 +143,7 @@ CREATE TABLE IF NOT EXISTS `pulls` ( `date_updated` int unsigned DEFAULT NULL, `date_closed` int unsigned DEFAULT NULL, `date_merged` int unsigned DEFAULT NULL, + `mergeable` tinyint(1) DEFAULT NULL, `milestone_title` varchar(255) COLLATE utf8mb4_general_ci DEFAULT NULL, `milestone_due_on` int unsigned DEFAULT NULL, `closes` int unsigned DEFAULT NULL, diff --git a/models/db_pull.js b/models/db_pull.js index 631a005e..6e8d67d3 100644 --- a/models/db_pull.js +++ b/models/db_pull.js @@ -17,6 +17,7 @@ function DBPull(pull) { date_updated: utils.toUnixTime(pullData.updated_at), date_closed: utils.toUnixTime(pullData.closed_at), date_merged: utils.toUnixTime(pullData.merged_at), + mergeable: pullData.mergeable, difficulty: pullData.difficulty, milestone_title: pullData.milestone.title, milestone_due_on: utils.toUnixTime(pullData.milestone.due_on), diff --git a/models/pull.js b/models/pull.js index 5c3cdb9f..d32d2edc 100644 --- a/models/pull.js +++ b/models/pull.js @@ -193,6 +193,7 @@ Pull.fromGithubApi = function ( created_at: utils.fromDateString(data.created_at), updated_at: utils.fromDateString(data.updated_at), closed_at: utils.fromDateString(data.closed_at), + mergeable: data.mergeable, merged_at: utils.fromDateString(data.merged_at), difficulty: data.difficulty, milestone: { @@ -246,6 +247,7 @@ Pull.getFromDB = function ( created_at: utils.fromUnixTime(data.date), updated_at: utils.fromUnixTime(data.date_updated), closed_at: utils.fromUnixTime(data.date_closed), + mergeable: data.mergeable, merged_at: utils.fromUnixTime(data.date_merged), difficulty: data.difficulty, milestone: { From fa36d3a66320405bb0ec59d906d175f9fee9b825 Mon Sep 17 00:00:00 2001 From: jordycosta Date: Wed, 8 Mar 2023 15:19:46 -0700 Subject: [PATCH 11/22] add a new migration file This adds in the 0018-add-mergeable-column.sql file to the /migrations folder in Pulldasher. --- migrations/0018-add-mergeable-column.sql | 1 + 1 file changed, 1 insertion(+) create mode 100644 migrations/0018-add-mergeable-column.sql diff --git a/migrations/0018-add-mergeable-column.sql b/migrations/0018-add-mergeable-column.sql new file mode 100644 index 00000000..91e54329 --- /dev/null +++ b/migrations/0018-add-mergeable-column.sql @@ -0,0 +1 @@ +ALTER TABLE `pulls` ADD COLUMN `mergeable` BOOLEAN default NULL AFTER `date_merged`; \ No newline at end of file From 5b2fc25d3c834fe010f7fbedafc44f013dbe93b0 Mon Sep 17 00:00:00 2001 From: jordycosta Date: Wed, 8 Mar 2023 19:16:48 -0700 Subject: [PATCH 12/22] update migration file This updates the migration to add the `mergeable` column by setting the datatype from BOOLEAN to tinyint --- migrations/0018-add-mergeable-column.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/migrations/0018-add-mergeable-column.sql b/migrations/0018-add-mergeable-column.sql index 91e54329..d461bf98 100644 --- a/migrations/0018-add-mergeable-column.sql +++ b/migrations/0018-add-mergeable-column.sql @@ -1 +1 @@ -ALTER TABLE `pulls` ADD COLUMN `mergeable` BOOLEAN default NULL AFTER `date_merged`; \ No newline at end of file +ALTER TABLE `pulls` ADD COLUMN `mergeable` tinyint default NULL AFTER `date_merged`; \ No newline at end of file From f164a3164fadc259b49450813c968e168a4df4f5 Mon Sep 17 00:00:00 2001 From: jordycosta <95656772+jordycosta@users.noreply.github.com> Date: Thu, 9 Mar 2023 08:58:46 -0800 Subject: [PATCH 13/22] update hasMergeConflicts method call hasMergeConflicts() is a method, so we should treat it as such Co-authored-by: Daniel Beardsley --- frontend/src/pulldasher/index.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/src/pulldasher/index.tsx b/frontend/src/pulldasher/index.tsx index 4c0c8a53..85515c59 100644 --- a/frontend/src/pulldasher/index.tsx +++ b/frontend/src/pulldasher/index.tsx @@ -33,7 +33,7 @@ function Pulldasher() { const allPulls = useAllPulls(); const pulls = useAllOpenPulls(); const pullsCIBlocked = pulls.filter((pull) => pull.isCiBlocked()); - const pullsDeployBlocked = pulls.filter((pull) => pull.isDeployBlocked() || pull.hasMergeConflicts); + const pullsDeployBlocked = pulls.filter((pull) => pull.isDeployBlocked() || pull.hasMergeConflicts()); const pullsReady = pulls.filter( (pull) => pull.isReady() && pull.isCiRequired() && !pull.hasMergeConflicts() ); From ebefde33fa365322d2ef7a0faaff59cea83f46e3 Mon Sep 17 00:00:00 2001 From: jordycosta Date: Thu, 9 Mar 2023 10:01:19 -0700 Subject: [PATCH 14/22] augment isReady() function This adds to the logic behind the isReady() function by asserting that a pull request is not considered "Ready" if merge conflicts are present. --- frontend/src/pull.ts | 3 ++- frontend/src/pulldasher/index.tsx | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/frontend/src/pull.ts b/frontend/src/pull.ts index 7a5c4778..d07de63a 100644 --- a/frontend/src/pull.ts +++ b/frontend/src/pull.ts @@ -106,7 +106,8 @@ export class Pull extends PullData { return ( this.hasMetDeployRequirements() && !this.getDevBlock() && - !this.getDeployBlock() + !this.getDeployBlock() && + !this.hasMergeConflicts() ); } diff --git a/frontend/src/pulldasher/index.tsx b/frontend/src/pulldasher/index.tsx index 85515c59..0bedfd8e 100644 --- a/frontend/src/pulldasher/index.tsx +++ b/frontend/src/pulldasher/index.tsx @@ -35,7 +35,7 @@ function Pulldasher() { const pullsCIBlocked = pulls.filter((pull) => pull.isCiBlocked()); const pullsDeployBlocked = pulls.filter((pull) => pull.isDeployBlocked() || pull.hasMergeConflicts()); const pullsReady = pulls.filter( - (pull) => pull.isReady() && pull.isCiRequired() && !pull.hasMergeConflicts() + (pull) => pull.isReady() && pull.isCiRequired() ); const pullsDevBlocked = pulls.filter( (pull) => pull.getDevBlock() || pull.isDraft() From 88db70109f0aa106040bd2afe84d455b2bd26df4 Mon Sep 17 00:00:00 2001 From: jordycosta Date: Thu, 9 Mar 2023 10:04:04 -0700 Subject: [PATCH 15/22] remove mergeConflict const This removes the mergeConflict flag const and opts to directly use the value "pull.hasMergeConflicts()" This is more direct and probably easier to read/understand --- frontend/src/pull-card/flags.tsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/frontend/src/pull-card/flags.tsx b/frontend/src/pull-card/flags.tsx index 58c5014d..59da0b2d 100644 --- a/frontend/src/pull-card/flags.tsx +++ b/frontend/src/pull-card/flags.tsx @@ -22,7 +22,6 @@ export const Flags = memo(function Flags({ pull }: { pull: Pull }) { const QAing = pull.getLabel("QAing"); const externalBlock = pull.getLabel("external_block"); const cryogenicStorage = pull.getLabel("Cryogenic Storage"); - const mergeConflict = pull.hasMergeConflicts(); return ( <> {deployBlock && ( @@ -88,7 +87,7 @@ export const Flags = memo(function Flags({ pull }: { pull: Pull }) { icon={faSnowflake} /> )} - {mergeConflict && ( + {pull.hasMergeConflicts() && ( Date: Thu, 9 Mar 2023 11:18:56 -0700 Subject: [PATCH 16/22] append to pull-card-demo This adds in the latest flags (draft + merge conflict) to the pull card demo section on Pulldasher Dev --- frontend/test/named-pulls.ts | 13 +++++++++++++ frontend/test/pull-card-demo.tsx | 1 + 2 files changed, 14 insertions(+) diff --git a/frontend/test/named-pulls.ts b/frontend/test/named-pulls.ts index 4a07bb1b..1a127615 100644 --- a/frontend/test/named-pulls.ts +++ b/frontend/test/named-pulls.ts @@ -290,6 +290,10 @@ export const Blocked = [ dev_block: [devBlock], }, }), + pullData({ + title: "Merge Conflict(s)", + mergeable: false, + }), ]; export const Milestones = [ @@ -342,6 +346,13 @@ export const Labels = [ }), ]; +export const Draft = [ + pullData({ + title: "Draft", + draft: true, + }), +]; + export const MyOwn = [ pullData({ title: "Pull Created By Me", @@ -371,6 +382,8 @@ export const KitchenSink = [ user: { login: getUser(), }, + mergeable: false, + draft: true, cr_req: 3, qa_req: 2, status: { diff --git a/frontend/test/pull-card-demo.tsx b/frontend/test/pull-card-demo.tsx index 95243083..07f3ef19 100644 --- a/frontend/test/pull-card-demo.tsx +++ b/frontend/test/pull-card-demo.tsx @@ -45,6 +45,7 @@ function PullCardDemo() { + From c090fa835302a32dadf86b13143bf6e9365e07b9 Mon Sep 17 00:00:00 2001 From: jordycosta Date: Thu, 9 Mar 2023 11:21:21 -0700 Subject: [PATCH 17/22] add import This adds the named-pulls.ts Draft const to pull-card-demo.tsx --- frontend/test/pull-card-demo.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/frontend/test/pull-card-demo.tsx b/frontend/test/pull-card-demo.tsx index 07f3ef19..ca9a9461 100644 --- a/frontend/test/pull-card-demo.tsx +++ b/frontend/test/pull-card-demo.tsx @@ -14,6 +14,7 @@ import { Blocked, Milestones, Labels, + Draft, MyOwn, KitchenSink, } from "./named-pulls"; From a54ee3edb4b2b7cddd3952c5bd9da8dfe34210d1 Mon Sep 17 00:00:00 2001 From: jordycosta Date: Thu, 9 Mar 2023 11:27:45 -0700 Subject: [PATCH 18/22] add draft + mergeable field to pull-date-parts Adds in the latest Pulldasher fields to the pullData return function --- frontend/test/pull-data-parts.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/frontend/test/pull-data-parts.ts b/frontend/test/pull-data-parts.ts index 47747492..2689a6be 100644 --- a/frontend/test/pull-data-parts.ts +++ b/frontend/test/pull-data-parts.ts @@ -79,12 +79,14 @@ export function pullData(p: DeepPartial): PullData { repoSpec: p.repoSpec || null, number: pullNumber(), state: "open", + draft: false, title: p.title || "Young pull with no CR / QA", body: "pull request dummy body", created_at: p.created_at || daysAgo(0), updated_at: daysAgo(0), closed_at: null, merged_at: null, + mergeable: null, difficulty: null, milestone: p.milestone || { title: null, From 4702c40edf9d80fccfb6dcb0bb11f0baa7d3cb08 Mon Sep 17 00:00:00 2001 From: jordycosta Date: Thu, 9 Mar 2023 13:02:55 -0700 Subject: [PATCH 19/22] update isDeployBlocked() logic We only want to send pulls requests with merge conflicts IFF CR/QA requirements are fulfilled --- frontend/src/pull.ts | 2 +- frontend/src/pulldasher/index.tsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/frontend/src/pull.ts b/frontend/src/pull.ts index d07de63a..0cee0ba9 100644 --- a/frontend/src/pull.ts +++ b/frontend/src/pull.ts @@ -115,7 +115,7 @@ export class Pull extends PullData { return ( this.hasMetDeployRequirements() && !this.getDevBlock() && - (!!this.getDeployBlock() || !this.isCiRequired()) + (!!this.getDeployBlock() || !this.isCiRequired() || this.hasMergeConflicts()) ); } diff --git a/frontend/src/pulldasher/index.tsx b/frontend/src/pulldasher/index.tsx index 0bedfd8e..db3e533d 100644 --- a/frontend/src/pulldasher/index.tsx +++ b/frontend/src/pulldasher/index.tsx @@ -33,7 +33,7 @@ function Pulldasher() { const allPulls = useAllPulls(); const pulls = useAllOpenPulls(); const pullsCIBlocked = pulls.filter((pull) => pull.isCiBlocked()); - const pullsDeployBlocked = pulls.filter((pull) => pull.isDeployBlocked() || pull.hasMergeConflicts()); + const pullsDeployBlocked = pulls.filter((pull) => pull.isDeployBlocked()); const pullsReady = pulls.filter( (pull) => pull.isReady() && pull.isCiRequired() ); From f0327e3758c3bfe3b6dad2dc18ec50b83b8630dc Mon Sep 17 00:00:00 2001 From: jordycosta Date: Thu, 9 Mar 2023 21:08:06 -0700 Subject: [PATCH 20/22] update flag color, pull-data-parts.ts This updates the merge conflict flag color from yellow to red This also updates the values being set for the 'draft' and 'mergeable' field on pull-data-parts.ts --- frontend/src/theme/day_theme.less | 2 +- frontend/src/theme/night_theme.less | 2 +- frontend/test/pull-data-parts.ts | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/frontend/src/theme/day_theme.less b/frontend/src/theme/day_theme.less index 704ac184..b30fdc64 100644 --- a/frontend/src/theme/day_theme.less +++ b/frontend/src/theme/day_theme.less @@ -133,7 +133,7 @@ body[data-theme="day_theme"] { --tag-cryo-border: var(--tag-default-border); --tag-cryo-background: @blue; - --tag-merge-conflict: @yellow; + --tag-merge-conflict: @red; --tag-merge-conflict-border: var(--tag-default-border); --tag-merge-conflict-background: var(--tag-default-background); diff --git a/frontend/src/theme/night_theme.less b/frontend/src/theme/night_theme.less index a30cf87e..1856e5ba 100644 --- a/frontend/src/theme/night_theme.less +++ b/frontend/src/theme/night_theme.less @@ -129,7 +129,7 @@ body[data-theme="night_theme"] { --tag-externally-blocked-border: var(--tag-default-border); --tag-externally-blocked-background: var(--tag-default-background); - --tag-merge-conflict: @yellow; + --tag-merge-conflict: @red; --tag-merge-conflict-border: var(--tag-default-border); --tag-merge-conflict-background: var(--tag-default-background); diff --git a/frontend/test/pull-data-parts.ts b/frontend/test/pull-data-parts.ts index 2689a6be..254b8758 100644 --- a/frontend/test/pull-data-parts.ts +++ b/frontend/test/pull-data-parts.ts @@ -79,14 +79,14 @@ export function pullData(p: DeepPartial): PullData { repoSpec: p.repoSpec || null, number: pullNumber(), state: "open", - draft: false, + draft: p.draft, title: p.title || "Young pull with no CR / QA", body: "pull request dummy body", created_at: p.created_at || daysAgo(0), updated_at: daysAgo(0), closed_at: null, merged_at: null, - mergeable: null, + mergeable: p.mergeable, difficulty: null, milestone: p.milestone || { title: null, From 479ce5d0c376db5903f26b11b1551bee2c0d5b9f Mon Sep 17 00:00:00 2001 From: jordycosta Date: Thu, 9 Mar 2023 21:24:08 -0700 Subject: [PATCH 21/22] update Deploy Blocked column sorting This adds sorting for the Deploy Blocked column to prioritize pull requests with the deploy_block label above pull requests with merge conflicts --- frontend/src/pulldasher/index.tsx | 4 ++-- frontend/src/pulldasher/sort.ts | 7 +++++++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/frontend/src/pulldasher/index.tsx b/frontend/src/pulldasher/index.tsx index db3e533d..68dfb5f8 100644 --- a/frontend/src/pulldasher/index.tsx +++ b/frontend/src/pulldasher/index.tsx @@ -1,7 +1,7 @@ import { useAllPulls, useAllOpenPulls } from "./pulls-context"; import { Navbar } from "../navbar"; import { Column } from "../column"; -import { QACompare } from "./sort"; +import { QACompare, DeployCompare } from "./sort"; import { LeaderList, getLeaders } from "../leader-list"; import { useMyPullNotification, @@ -76,7 +76,7 @@ function Pulldasher() { id="dep" title="Deploy Blocked" variant="deployBlocked" - pulls={pullsDeployBlocked} + pulls={pullsDeployBlocked.sort(DeployCompare)} /> diff --git a/frontend/src/pulldasher/sort.ts b/frontend/src/pulldasher/sort.ts index eb405de4..9846a91c 100644 --- a/frontend/src/pulldasher/sort.ts +++ b/frontend/src/pulldasher/sort.ts @@ -30,6 +30,13 @@ export function QACompare(a: Pull, b: Pull): number { ); } +export function DeployCompare(a: Pull, b: Pull): number { + return ( + // Pulls with no merge conflicts above those with merge conflicts + compareBool(!a.hasMergeConflicts(), !b.hasMergeConflicts()) + ); +} + export function signatureCompare(a: Signature, b: Signature) { return ( // Active before inactive From 9302787e210dbf18761216e67cb4398c20fc9c97 Mon Sep 17 00:00:00 2001 From: jordycosta Date: Thu, 9 Mar 2023 21:29:19 -0700 Subject: [PATCH 22/22] update flag color (x2) Changes the flag color for the mergeConflict variant from red to yellow I think that this yellow is a little more subtle and indicates less importance in comparison to higher priority flags (dev_block, deploy_block, etc.) --- frontend/src/theme/day_theme.less | 2 +- frontend/src/theme/night_theme.less | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/frontend/src/theme/day_theme.less b/frontend/src/theme/day_theme.less index b30fdc64..704ac184 100644 --- a/frontend/src/theme/day_theme.less +++ b/frontend/src/theme/day_theme.less @@ -133,7 +133,7 @@ body[data-theme="day_theme"] { --tag-cryo-border: var(--tag-default-border); --tag-cryo-background: @blue; - --tag-merge-conflict: @red; + --tag-merge-conflict: @yellow; --tag-merge-conflict-border: var(--tag-default-border); --tag-merge-conflict-background: var(--tag-default-background); diff --git a/frontend/src/theme/night_theme.less b/frontend/src/theme/night_theme.less index 1856e5ba..a30cf87e 100644 --- a/frontend/src/theme/night_theme.less +++ b/frontend/src/theme/night_theme.less @@ -129,7 +129,7 @@ body[data-theme="night_theme"] { --tag-externally-blocked-border: var(--tag-default-border); --tag-externally-blocked-background: var(--tag-default-background); - --tag-merge-conflict: @red; + --tag-merge-conflict: @yellow; --tag-merge-conflict-border: var(--tag-default-border); --tag-merge-conflict-background: var(--tag-default-background);