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

docs(src-cli): update src-cli commands to use 'metadata' instead of 'kvp' for consistency and clarity#51031

Merged
erzhtor merged 1 commit into
mainfrom
erzhtor/rename-src-cli-kvp-to-metadata
Apr 24, 2023
Merged

docs(src-cli): update src-cli commands to use 'metadata' instead of 'kvp' for consistency and clarity#51031
erzhtor merged 1 commit into
mainfrom
erzhtor/rename-src-cli-kvp-to-metadata

Conversation

@erzhtor

@erzhtor erzhtor commented Apr 24, 2023

Copy link
Copy Markdown
Contributor

Part of https://github.com/sourcegraph/pr-faqs/issues/96. Follow-up sourcegraph/src-cli#972.

Test plan

  • Check the diff

@erzhtor erzhtor self-assigned this Apr 24, 2023
@cla-bot cla-bot Bot added the cla-signed label Apr 24, 2023
@sourcegraph-bot

sourcegraph-bot commented Apr 24, 2023

Copy link
Copy Markdown
Contributor

Codenotify: Notifying subscribers in CODENOTIFY files for diff ff9af8b...702a3b2.

Notify File(s)
@sourcegraph/delivery doc/admin/repo/metadata.md

@erzhtor erzhtor added the merge-on-any-approve Whomever approves the PR can merge without waiting for the author label Apr 24, 2023
kopancek
kopancek approved these changes Apr 24, 2023
View reviewed changes

@kopancek kopancek left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not sure where the src-cli change is, but it would be good to keep backwards compatibility for add-kvp, etc. In case there are already scripts created using that option, so that people do not need to change the scripts.

For docs, I think it is good to only mention the new way of doing things.

@erzhtor

erzhtor commented Apr 24, 2023

Copy link
Copy Markdown
Contributor Author

Not sure where the src-cli change is, but it would be good to keep backwards compatibility for add-kvp, etc. In case there are already scripts created using that option, so that people do not need to change the scripts.

For docs, I think it is good to only mention the new way of doing things.

Thanks for the review, @kopancek. Yes, the src-cli keeps old -kvp commands too with deprecation mentions.

@erzhtor erzhtor merged commit bb3b0c3 into main Apr 24, 2023
@erzhtor erzhtor deleted the erzhtor/rename-src-cli-kvp-to-metadata branch April 24, 2023 09:05
sashaostrikov
sashaostrikov reviewed Apr 24, 2023
View reviewed changes

@sashaostrikov sashaostrikov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Had this PR opened for way too long and now it's merged :D
Good job!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@toolmantim toolmantim Awaiting requested review from toolmantim

@limitedmage limitedmage Awaiting requested review from limitedmage

@ryphil ryphil Awaiting requested review from ryphil

@camdencheek camdencheek Awaiting requested review from camdencheek

2 more reviewers

@sashaostrikov sashaostrikov sashaostrikov left review comments

@kopancek kopancek kopancek approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@erzhtor erzhtor

Labels

cla-signed merge-on-any-approve Whomever approves the PR can merge without waiting for the author

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@erzhtor @sourcegraph-bot @kopancek @sashaostrikov