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

(fix) add repo meta create & update user-friendly error message#51985

Merged
erzhtor merged 7 commits intomainfrom
erzhtor/add-repo-meta-create-update-user-friendly-error-messages
May 18, 2023
Merged

(fix) add repo meta create & update user-friendly error message#51985
erzhtor merged 7 commits intomainfrom
erzhtor/add-repo-meta-create-update-user-friendly-error-messages

Conversation

@erzhtor
Copy link
Contributor

@erzhtor erzhtor commented May 16, 2023

Part of https://github.com/sourcegraph/pr-faqs/issues/96.

Test plan

Screenshots

Description Before After
Create duplicate key image image
Update non-existing key-value pair. This is mainly used by src repos update-metadata command image image

@erzhtor erzhtor requested review from a team, camdencheek, ryphil and toolmantim May 16, 2023 12:15
@erzhtor erzhtor self-assigned this May 16, 2023
@cla-bot cla-bot bot added the cla-signed label May 16, 2023
Copy link
Contributor

@sashaostrikov sashaostrikov left a comment

Choose a reason for hiding this comment

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

I proposed the other way around for checking that there is already an existing metadata key, PTAL

@erzhtor erzhtor requested review from a team and sashaostrikov May 17, 2023 07:47
Copy link
Contributor

@sashaostrikov sashaostrikov left a comment

Choose a reason for hiding this comment

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

PTAL at my comment, I guess I caused some confusion with my last review, sorry!

@erzhtor erzhtor force-pushed the erzhtor/add-repo-meta-create-update-user-friendly-error-messages branch from 2bcec90 to e47b030 Compare May 18, 2023 08:49
@erzhtor erzhtor requested review from kopancek and sashaostrikov May 18, 2023 08:52
Copy link
Contributor

@sashaostrikov sashaostrikov left a comment

Choose a reason for hiding this comment

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

:shipit:

@erzhtor erzhtor enabled auto-merge (squash) May 18, 2023 12:17
@erzhtor erzhtor disabled auto-merge May 18, 2023 12:17
Copy link
Contributor

@toolmantim toolmantim left a comment

Choose a reason for hiding this comment

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

Much nicer!! Minor suggestion is to either remove "Repo", or use the full word "Repository" at the beginning and remove "for the given repository" at the end

erzhtor and others added 2 commits May 18, 2023 20:30
Co-authored-by: Tim Lucas <t@toolmantim.com>
Co-authored-by: Tim Lucas <t@toolmantim.com>
erzhtor and others added 2 commits May 18, 2023 22:37
Co-authored-by: Camden Cheek <camden@ccheek.com>
(chore): add scanKVP and scanKVPs reusable helper functions
@erzhtor erzhtor enabled auto-merge (squash) May 18, 2023 17:04
@erzhtor erzhtor disabled auto-merge May 18, 2023 17:20
@erzhtor erzhtor enabled auto-merge (squash) May 18, 2023 17:21
@erzhtor
Copy link
Contributor Author

erzhtor commented May 18, 2023

@kopancek, looks like I can't merge PR unless you stamp it?

@erzhtor erzhtor requested review from kopancek and removed request for kopancek May 18, 2023 17:22
@erzhtor erzhtor merged commit ef40256 into main May 18, 2023
@erzhtor erzhtor deleted the erzhtor/add-repo-meta-create-update-user-friendly-error-messages branch May 18, 2023 20:59
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants