Skip to content

Conversation

ashalfarhan
Copy link
Contributor

@ashalfarhan ashalfarhan commented Dec 20, 2021

🎉 Thanks for submitting a pull request! 🎉

Summary

Fixes #3575

This PR will add the ability to transfer environment variables from one site to another.
If you want to send env vars from one site to another you can run this subcommand.
Given 2 arguments, the basic usage is:

  • If both arguments are provided, then the first argument will be the sender, and the second one is the destination
  • If only one argument is provided, then that will be the destination, and the sender should be the current directory site, otherwise, it will exit with a helpful error message
  • If there's no env from siteIdA, then it will exit because nothing to transfer

So users can use this command without opening their browser to copy and paste the env vars from one to another.

Notes:

  • Currently this just only works with site id.

For us to review and ship your PR efficiently, please perform the following steps:

  • Open a bug/issue before writing your code 🧑‍💻. This ensures we can discuss the changes and get feedback from everyone that should be involved. If you`re fixing a typo or something that`s on fire 🔥 (e.g. incident related), you can skip this step.
  • Read the contribution guidelines 📖. This ensures your code follows our style guide and
    passes our tests.
  • Update or add tests (if any source code was changed or added) 🧪
  • Update or add documentation (if features were changed or added) 📝
  • Make sure the status checks below are successful ✅

A picture of a cute animal (not mandatory, but encouraged)

koala5

@ashalfarhan ashalfarhan requested a review from a team as a code owner December 20, 2021 19:29
@ashalfarhan ashalfarhan changed the title Feature/env transfer feat: env:transfer subcommand Dec 20, 2021
@ashalfarhan ashalfarhan changed the title feat: env:transfer subcommand feat(command-env): add transfer subcommand Dec 20, 2021
@erezrokah erezrokah added the type: feature code contributing to the implementation of a feature and/or user facing functionality label Dec 21, 2021
@erezrokah erezrokah self-requested a review December 21, 2021 10:46
@erezrokah erezrokah self-assigned this Dec 21, 2021
@erezrokah erezrokah force-pushed the feature/env-transfer branch from da4bae9 to 62718d5 Compare December 28, 2021 18:15
Copy link
Contributor

@erezrokah erezrokah left a comment

Choose a reason for hiding this comment

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

@ashalfarhan, sorry for the very later review.

I changed the command a bit to use options instead of arguments.
It makes the purpose of each site id more explicit.
I also added error handling to handle when the CLI can't get the site from the id.
Finally, I think transfer can be confusing for this command, as it doesn't remove the variables from the from site (I should have figured this out sooner).

We could try calling it env:migrate or env:clone, what do you think?

@ashalfarhan
Copy link
Contributor Author

@ashalfarhan, sorry for the very later review.

I changed the command a bit to use options instead of arguments. It makes the purpose of each site id more explicit. I also added error handling to handle when the CLI can't get the site from the id. Finally, I think transfer can be confusing for this command, as it doesn't remove the variables from the from site (I should have figured this out sooner).

We could try calling it env:migrate or env:clone, what do you think?

Yeah, I think it's better to use options instead of arguments.
And sorry to make you work for the error handling 🙏, because I am still not sure should I just log the error or I should do something, so I just let the error be thrown.

Btw env:migrate is more relevant for this feature I think.

@ashalfarhan ashalfarhan changed the title feat(command-env): add transfer subcommand feat(command-env): add migrate subcommand Dec 28, 2021
Copy link
Contributor

@erezrokah erezrokah left a comment

Choose a reason for hiding this comment

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

🎸

@erezrokah erezrokah added the automerge Add to Kodiak auto merge queue label Dec 29, 2021
@kodiakhq kodiakhq bot merged commit 290f9a8 into netlify:main Dec 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automerge Add to Kodiak auto merge queue type: feature code contributing to the implementation of a feature and/or user facing functionality

2 participants