-
Notifications
You must be signed in to change notification settings - Fork 42
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
🌱 Setup branding to be configurable during the build (#1664, #1742) #1738
Merged
Conversation
This file contains 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
sjd78
approved these changes
Mar 5, 2024
Probably will have to get this in for 7.0.3 since it is a non-blocker. |
I'll setup a full backport with this PR and #1742 rolled together. |
Reopening for v0.3.1 I'll update the github actions files to current for the release-0.3 branch in another PR. |
sjd78
added
the
do-not-merge/hold
Indicates that a PR should not merge because someone has issued a /hold command.
label
Mar 14, 2024
## Summary Refactor and enhance branding capabilities. Now branding is included from a single directory that contains at least `strings.json`, `manifest.json`, and `favicon.ico`. Any other assets may be placed in the directory and will be copied to the bundled app. Running the build as normal should show no difference. Running the build with `BRANDING=./some-other-brand npm run start:dev` will try to use a branding from the specified directory (that is relative to the project's source root). See `BRANDING.md` for more details. Resolves: #1682 ## Details Branding changes: - Move Konveyor branding assets to a project top-level branding directory - Remove MTA branding assets - Remove profile/branding constants from `env` and client module - Embed branding strings and assets in the common module - `strings.json` is templated to allow the build to adjust asset URL path roots as necessary - `brandingAssetPath()` - server's index.html generation sources the template strings from the common module's branding strings - `HeaderApp` and `AppAboutModal` components support branding by using the `useBranding` hook - `BRANDING` as a relative path is computed from the project root - webpack build source branding assets directly from the common module - Unit tests, snapshots and jest configs updated as necessary Jest changes: - Use `react-i18next` mock from `client/__mocks__` as a more robust mock borrowed from react-i18n repos - Move `setupTests.ts` into `client/src/app/test-config` to keep jest test config code all in the same directory Related changes: - Upgrade rollup to v4, add new rollup plugins (copy, virtual) --------- Signed-off-by: Scott J Dickerson <[email protected]> Co-authored-by: Ian Bolton <[email protected]> Signed-off-by: Cherry Picker <[email protected]>
Following up on PRs #1664 and #1739, downgrade to `rollup@^3.0`. The v4 of rollup has "native" code that runs the build, and that code does not currently work with s390 builds [1]. Since konveyor is built for x86, arm, ppc, and s390, all of those need to work. When the s390 builds can work with the wasm bits, we can upgrade to `rollup@^4.0` again. [1] - rollup/rollup#5354 Signed-off-by: Scott J Dickerson <[email protected]>
sjd78
force-pushed
the
cherry-pick-pr1664-release-0.3
branch
from
March 14, 2024 23:37
721a6c8
to
8ca2ef2
Compare
sjd78
changed the title
🌱 Setup branding to be configurable during the build (#1664)
🌱 Setup branding to be configurable during the build (#1664, #1742)
Mar 14, 2024
sjd78
removed
the
do-not-merge/hold
Indicates that a PR should not merge because someone has issued a /hold command.
label
Mar 14, 2024
ibolton336
approved these changes
Mar 15, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Refactor and enhance branding capabilities. Now branding is included
from a single directory that contains at least
strings.json
,manifest.json
, andfavicon.ico
. Any other assets may be placed inthe directory and will be copied to the bundled app.
Running the build as normal should show no difference.
Running the build with
BRANDING=./some-other-brand npm run start:dev
will try to use a branding from the specified directory (that is relative to the
project's source root).
See
BRANDING.md
for more details.Resolves: #1682
Details
Branding changes:
Move Konveyor branding assets to a project top-level branding directory
Remove MTA branding assets
Remove profile/branding constants from
env
and client moduleEmbed branding strings and assets in the common module
strings.json
is templated to allow the build to adjust assetURL path roots as necessary
brandingAssetPath()
server's index.html generation sources the template strings from the
common module's branding strings
HeaderApp
andAppAboutModal
components support branding by usingthe
useBranding
hookBRANDING
as a relative path is computed from the project rootwebpack build source branding assets directly from the common module
Unit tests, snapshots and jest configs updated as necessary
Jest changes:
Use
react-i18next
mock fromclient/__mocks__
as a more robust mockborrowed from react-i18n repos
Move
setupTests.ts
intoclient/src/app/test-config
to keep jesttest config code all in the same directory
Related changes:
Upgrade rollup to v4,add new rollup plugins (copy, virtual)Downgrade to rollup v3 for multiarch builds (#1742)
As a followup on PRs #1664 and #1739, it became necessary to downgrade
back to
rollup@^3.0
. The v4 of rollup has "native" code that runs the build, andthat code does not currently work with s390 builds [1]. Since konveyor is built
for x86, arm, ppc, and s390, all of those need to work.
When the s390 builds can work with the wasm bits, we can upgrade to
rollup@^4.0
again.[1] - rollup/rollup#5354
Signed-off-by: Scott J Dickerson [email protected]
Co-authored-by: Ian Bolton [email protected]
Signed-off-by: Cherry Picker [email protected]