Skip to content

ALL CHANGES#1

Open
sgc-code wants to merge 59 commits into
emptyfrom
main
Open

ALL CHANGES#1
sgc-code wants to merge 59 commits into
emptyfrom
main

Conversation

@sgc-code
Copy link
Copy Markdown
Contributor

@sgc-code sgc-code commented Mar 11, 2026

Comment thread package.json Outdated
Comment thread browser-shims/vite.js
Comment thread src/env.ts Outdated
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

maybe we should remove this thing, is it even working?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes, it is working and still useful and used

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

where?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ok, I removed the unused setEnv, but getEnv is still used at 2 places, however given that they both require different part of the env related stuff I wonder if it wouldn't be best to just move each where used.
Idk if best to have 1 place for all env related stuff or 2 places each getting what they need.

I'm more for leaving it as is now, but open to counter args

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

not sure what's the point of the env.ts file, it was created to allow injection of an env for things like the browser, but that requires lazy initialization. we might as well remove it completely and leave it as it was before

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I quite like the idea to have everything related to env in one file rather than scattered all around

Comment thread package.json Outdated
@gaetbout
Copy link
Copy Markdown
Contributor

Once agreed to not change anything, ping me so that I can make a release and upgrade all consumers to it

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.

2 participants