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

feat: add flag to skip env var fetching #6723

Merged
merged 3 commits into from
Jul 2, 2024
Merged

feat: add flag to skip env var fetching #6723

merged 3 commits into from
Jul 2, 2024

Conversation

davbree
Copy link
Contributor

@davbree davbree commented Jun 23, 2024

πŸŽ‰ Thanks for submitting a pull request! πŸŽ‰

Summary

Introduce internal option --offline-env to disable using any env vars fetched from the API.
This offers better support for the case of receiving all env vars externally (via devbot).

ref CRE-1078


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)

@davbree davbree requested a review from a team as a code owner June 23, 2024 06:24
Copy link

github-actions bot commented Jun 23, 2024

πŸ“Š Benchmark results

Comparing with a265859

  • Dependency count: 1,212 (no change)
  • Package size: 313 MB (no change)
  • Number of ts-expect-error directives: 976 (no change)

@lukasholzer
Copy link
Collaborator

why cannot we use the existing --offline flag for it?

@davbree
Copy link
Contributor Author

davbree commented Jul 1, 2024

We still want to fetch the site config (for the dev command and target port). I could provide that from bitballoon as well in the future (CRE-1431) and then we can use --offline (and remove --offline-env)

@lukasholzer
Copy link
Collaborator

We still want to fetch the site config (for the dev command and target port). I could provide that from bitballoon as well in the future (CRE-1431) and then we can use --offline (and remove --offline-env)

As this is public API on the CLI we should provide that probably first to be able to leverage the --offline flag.

@mraerino
Copy link
Member

mraerino commented Jul 1, 2024

i think there is value in having a flag just for "don't fetch env vars". using --offline means that no interaction can happen, but we might want the CLI to talk to services regardless of if it is supposed to fetch env vars

@davbree
Copy link
Contributor Author

davbree commented Jul 1, 2024

I'm using hideHelp(true) to keep things internal (like we do for other parameters). Makes it easier to do this not all together..

@davbree davbree enabled auto-merge (squash) July 2, 2024 06:35
@davbree davbree merged commit a0f33c1 into main Jul 2, 2024
48 checks passed
@davbree davbree deleted the offline-env branch July 2, 2024 07:37
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.

None yet

4 participants