Skip to content

Conversation

@bobheadxi
Copy link
Member

@bobheadxi bobheadxi commented May 29, 2020

closes https://github.com/sourcegraph/sourcegraph/issues/10419

looks like ConfigurationSubject was removed a while back, and SettingsSubject has replaced it

SettingsSubject is something that can have settings: a site ("global settings", which is different from "site configuration"), an organization, or a user.

go build ./cmd/src
./src config list
./src config get

a variety of other Configuration-related fields have been listed as deprecated, so I've gone ahead and migrated them to use Settings

Copy link
Contributor

@mrnugget mrnugget left a comment

Choose a reason for hiding this comment

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

Approve to unblock, but I think before you merge, you should rename the other variable names too so they reflect the new state.


const configurationSubjectFragment = `
fragment ConfigurationSubjectFields on ConfigurationSubject {
fragment ConfigurationSubjectFields on SettingsSubject {
Copy link
Contributor

Choose a reason for hiding this comment

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

To avoid others being confused by this in the future, let's make the naming consistent 🙂

Suggested change
fragment ConfigurationSubjectFields on SettingsSubject {
fragment SettingsSubjectFields on SettingsSubject {

Copy link
Member Author

@bobheadxi bobheadxi Jun 2, 2020

Choose a reason for hiding this comment

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

I dug a bit deeper and the changes turned out to be pretty far-reaching - all the Configuration fields are listed as deprecated, and the shape of some things have changed, so I went ahead and made everything Settings: 9a39f3e

@bobheadxi bobheadxi requested a review from mrnugget June 2, 2020 16:53
@bobheadxi bobheadxi changed the title fix configuration subject fragment migrate Configuration usage to Settings Jun 2, 2020
Copy link
Member

@emidoots emidoots left a comment

Choose a reason for hiding this comment

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

Nice work!

@emidoots
Copy link
Member

emidoots commented Jun 2, 2020

Once you merge, please create a release following these instructions: https://github.com/sourcegraph/src-cli/blob/master/DEVELOPMENT.md#releasing

@bobheadxi bobheadxi merged commit 97dd215 into master Jun 3, 2020
@bobheadxi bobheadxi deleted the fix-config branch June 3, 2020 01:18
@mrnugget
Copy link
Contributor

mrnugget commented Jun 3, 2020

Good stuff, thanks @bobheadxi!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

src config commands fail

4 participants