Skip to content

Conversation

@ryanslade
Copy link
Contributor

This change removes the -endpoint flag

It also ensures the following:

  • Both env vars override the config file.
  • If a config file exists, either zero or both env vars must be
    specified. It is an error if only one is specified. This is
    to avoid confusion; we never merge config from the file and env vars.

Closes: #222

@ryanslade
Copy link
Contributor Author

ryanslade commented Jun 10, 2020

Some of our LSIF docs mentions -endpoint so we'll need to update that after this is released

This change removes the -endpoint flag

It also ensures the following:

* Both env vars override the config file.
* If a config file exists, either zero or both env vars must be
  specified. It is an error if only one is specified. This is
  to avoid confusion; we never merge config from the file and env vars.
@ryanslade ryanslade marked this pull request as ready for review June 10, 2020 12:43
@ryanslade ryanslade requested review from emidoots and mrnugget June 10, 2020 12:44
@emidoots
Copy link
Member

This will likely break some admins’ usage of this script, which they may not have any good visibility into. We’ll need to think about: how do we communicate breaking changes to the CLI? Before merging this

@emidoots
Copy link
Member

Adding more color to my comment: I think just documenting this in a new CHANGELOG.md file would be good enough.

If you want to get more fancy, you could detect if -endpoint is passed in code and error out / say it is deprecated - but I don't think that's needed.

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.

Looks good! Agree with @slimsag: it would be cool if you can start with a CHANGELOG.md

@ryanslade
Copy link
Contributor Author

I've added a skeleton changelog, I assume we didn't want any historical changes in there?

@mrnugget
Copy link
Contributor

Yep, looks good!

@ryanslade ryanslade merged commit a167443 into master Jun 12, 2020
ryanslade added a commit that referenced this pull request Jun 26, 2020
This reverts commit a167443.

This broke some existing users of src-cli. Another PR will make this a
more graceful deprecation.
ryanslade added a commit that referenced this pull request Jun 26, 2020
* Revert "Remove -endpoint flag (#225)"

This reverts commit a167443.

This broke some existing users of src-cli. Another PR will make this a
more graceful deprecation.

* Remove unused package
scjohns pushed a commit that referenced this pull request Apr 24, 2023
* Remove -endpoint flag

This change removes the -endpoint flag

It also ensures the following:

* Both env vars override the config file.
* If a config file exists, either zero or both env vars must be
  specified. It is an error if only one is specified. This is
  to avoid confusion; we never merge config from the file and env vars.

* Add CHANGELOG.md

* Add another pending change
scjohns pushed a commit that referenced this pull request Apr 24, 2023
* Revert "Remove -endpoint flag (#225)"

This reverts commit a167443.

This broke some existing users of src-cli. Another PR will make this a
more graceful deprecation.

* Remove unused package
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.

Env var SRC_ENDPOINT is ignored when config file exists

4 participants