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.
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
DOP-5159 Specify content repo and parser version in build script for Netlify Frontend Builds #1309
DOP-5159 Specify content repo and parser version in build script for Netlify Frontend Builds #1309
Changes from all commits
0f6a377
d7cc504
90cd72d
43ae22c
f64cf26
b39f5f7
e9edfb1
6a5568f
0cd4396
118f1d1
9a221ad
5751781
bbcdea8
6d048ec
a8fac5f
38fcf6e
5f9b2b8
471bbb3
caf006d
ec4abd2
24e94ee
af4d17e
9b571ac
fba7e5c
57895c3
ff36663
a3b1c2b
20ca87b
9f2eccf
2c6e65e
50ff0e7
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Do we want to throw an error immediately if any of these are empty/undefined? Or do we want to have a fallback for the PARSER_VERSION and the TESTING_BRANCH_NAME? ie "latest" and "master"
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.
+1 for fallback defaults for these
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 can set the fallback for these values as environment variables in the UI, or throw an error if they're undefined in the toml. Which do you think is the better way to go @seungpark @mmeigs
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 feel like directly in the toml as fallbacks might be good to keep it clear !
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.
Sorry I'm not clear on what that would look like, would you mind elaborating? How is what you are suggesting different than what is currently in the toml?
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.
sorry, i meant directly in this shell script** but in the toml is fine. in the case we want to deploy multiple previews, is that possible?
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.
Hm, I believe Netlify only really supports one deploy preview per build, but I'll brainstorm more
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.
are we only testing docs-landing for now, or can we build all content sites with this change right away. Out of curiosity, which ones have we already tested for ?
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.
All of them should be able to be built! I've tested landing, docs-cpp, and docs-relational-migrator so far