-
Notifications
You must be signed in to change notification settings - Fork 103
fix: add missing build-time env vars #7063
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
Open
hrishikesh-k
wants to merge
19
commits into
main
Choose a base branch
from
hk/build-env
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
19 commits
Select commit
Hold shift + click to select a range
e71ec39
add missing env vars
hrishikesh-k cd8cbf3
fix config/tests/base/tests.js
hrishikesh-k c59475d
fix config/tests/parse/tests.js
hrishikesh-k 2a329aa
fix config/tests/validate/tests.js
hrishikesh-k f087473
fix config/tests/normalize/tests.js
hrishikesh-k 0fe886f
change slugify logic and fix config/tests/context/tests.js
hrishikesh-k fb7351b
fix config/tests/load/tests.js
hrishikesh-k 006bd71
fix config/tests/api/tests.js
hrishikesh-k 9947191
fix config/tests/env/tests.js
hrishikesh-k 84f7e92
fix config/tests/cli/tests.js
hrishikesh-k c9a1ed0
fix config/tests/cwd/tests.js
hrishikesh-k 3dcf2a3
fix config/tests/mutate/tests.js
hrishikesh-k c4c2be8
fix config/tests/log/tests.js
hrishikesh-k fcee809
fix lint
hrishikesh-k 7f92399
allow site_name to be overwritten and fix build/tests/monitor/tests.js
hrishikesh-k 4247e17
fix build/tests/core/tests.js
hrishikesh-k 9a2914d
switch dep
hrishikesh-k 777589c
fix build/tests/telemetry/tests.js
hrishikesh-k 4c3319f
downgrade @sindresorhus/transliterate
hrishikesh-k File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Binary file not shown.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Binary file not shown.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Binary file not shown.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be produced for those "mock" values?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think, yes. These lines are coming from here:
build/packages/build/src/error/monitor/location.js
Lines 13 to 21 in 777589c
DEPLOY_IDexisted,SITE_NAMEdidn't. NowSITE_NAMEalso exists resulting in the line existing. Based on the code, I think it makes sense to have these lines printed.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am a bit conflicted on this - not sure if one is worse then the other, but that
buildLogsurl won't ever work, so that's why I was wondering wether we should returnundefinedfor the mocked values case (as in expand conditions that are checked to avoid constructing url if values are mocked)But I guess same can be same about mocked
SITE_NAMEand all the others, that they won't work either in practice - so probably matter of degree on what make sense to "mock" and what does not 🤔