Skip to content

overrides get command - #278

Merged
rabiaq123 merged 15 commits into
mainfrom
DVC-8632-overrides-get
Sep 7, 2023
Merged

overrides get command#278
rabiaq123 merged 15 commits into
mainfrom
DVC-8632-overrides-get

Conversation

@rabiaq123

@rabiaq123 rabiaq123 commented Sep 5, 2023

Copy link
Copy Markdown
Contributor
  • dvc overrides get --feature <feature-key> --environment <environment-key>
  • use case: viewing a feature-specific override given the environment

@rabiaq123
rabiaq123 force-pushed the DVC-8632-overrides-get branch from 0456276 to 922ea61 Compare September 6, 2023 19:36
Comment thread src/api/zodClient.ts
Comment thread src/commands/overrides/get.ts Outdated
this.writer.showResults({ environment: environment.key, variation: null })
return
}
this.writer.showError(

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.

this should just be normal output, not showError

@rabiaq123
rabiaq123 marked this pull request as ready for review September 7, 2023 16:41
@rabiaq123 rabiaq123 changed the title WIP overrides get command overrides get command Sep 7, 2023
@rabiaq123
rabiaq123 requested a review from a team September 7, 2023 16:41
Comment thread src/commands/overrides/get.ts Outdated
export default class DetailedTargeting extends Base {
static hidden = false
authRequired = true
static description = 'View the overrides associated with your DevCycle Identity in your current project.'

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.

Suggested change
static description = 'View the overrides associated with your DevCycle Identity in your current project.'
static description = 'View the Overrides associated with your DevCycle Identity in your current project.'

Comment thread src/commands/overrides/get.ts Outdated
import { fetchEnvironmentByKey } from '../../api/environments'
import { fetchVariationByKey } from '../../api/variations'

export default class DetailedTargeting extends Base {

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.

should this be named something else?

Suggested change
export default class DetailedTargeting extends Base {
export default class DetailedOverrides extends Base {

Comment thread src/commands/overrides/get.ts Outdated
this.writer.showResults(
`Override for feature: ${featureKey} on environment: ${environment.key} is variation: <not-set>`
)
this.writer.infoMessageWithCommand('To set an override, use: ', 'dvc overrides update')

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.

space is not needed here, it's handled in the function

Suggested change
this.writer.infoMessageWithCommand('To set an override, use: ', 'dvc overrides update')
this.writer.infoMessageWithCommand('To set an override, use:', 'dvc overrides update')

Comment thread src/commands/overrides/get.ts Outdated
this.writer.showResults({ environment: environment.key, variation: null })
return
}
this.writer.showResults(

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.

let's use showRawResults here so it doesn't stringify the result again and add extra quotations
image

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.

you mean line 74, just to be clear?

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.

yup, in non-headless mode

@rabiaq123
rabiaq123 force-pushed the DVC-8632-overrides-get branch from 131d74f to ae5544f Compare September 7, 2023 21:33
@rabiaq123
rabiaq123 merged commit df90f56 into main Sep 7, 2023
@rabiaq123
rabiaq123 deleted the DVC-8632-overrides-get branch September 7, 2023 21:36
jonathannorris pushed a commit that referenced this pull request Aug 11, 2025
override get command logic for when a feature and environment are specified
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants