Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

♻️ Rename server address method / environment variable #256

Merged
merged 1 commit into from
Mar 22, 2021

Conversation

wwilsman
Copy link
Contributor

Purpose

Initially, the PERCY_CLI_API environment variable was named so because it was the CLI that set that variable for SDKs to consume. However, that API address can still be consumed by SDKs without the CLI. In reality, this value comes from the core server, so it's name should reflect that.

PERCY_SERVER_ADDRESS was chosen as it is pretty descriptive in that the variable contains the address of the local Percy server. However, I'm open to other suggestions. bikeshed

Since this variable is only set by the CLI, and official SDKs consume this internally, I wouldn't count this as breaking for our own SDKs. If the user was using the --port option (not likely), it will still continue to work once the SDK is updated to account for the new variable name. However this would be considered a breaking change for custom SDKs that do not use sdk-utils, although this package is technically still beta so changes should be expected.

Depends on #252, #253, #254, & #255

@wwilsman wwilsman added the 🧹 maintenance General maintenance label Mar 19, 2021
@wwilsman wwilsman requested a review from Robdel12 March 19, 2021 17:26
@wwilsman wwilsman force-pushed the ww/cors-exposed-headers branch from 834f6a5 to 68b99b9 Compare March 19, 2021 18:21
Base automatically changed from ww/cors-exposed-headers to master March 19, 2021 18:26
@wwilsman wwilsman force-pushed the ww/rename-server-addr-vars branch from 65c843a to bc5f45b Compare March 19, 2021 18:26
Copy link
Contributor

@Robdel12 Robdel12 left a comment

Choose a reason for hiding this comment

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

I dig it, PERCY_SERVER_ADDRESS is much better 🏁

@wwilsman wwilsman merged commit 61a7498 into master Mar 22, 2021
@wwilsman wwilsman deleted the ww/rename-server-addr-vars branch March 22, 2021 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🧹 maintenance General maintenance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants