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.

Sign up for GitHub

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

Jump to bottom

feat(user-settings): allow overriding build info in the user settings page #2564

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat(user-settings): allow overriding build info in the user settings page #2564

wants to merge 1 commit into from

Conversation

debsmita1
Copy link
Member

@debsmita1 debsmita1 commented Mar 12, 2025 •
edited
Loading

Description

  • Allowing users to specify their customized build info via app-config
  • Customized build shows first 2 information when collapsed
  • Fallbacks to build-metadata.json content when customization is not available

Which issue(s) does this PR fix

Screenshots

Screenshot 2025-03-13 at 12 00 39 PM

PR acceptance criteria

Please make sure that the following steps are complete:

  • GitHub Actions are completed and successful
  • Unit Tests are updated and passing
  • E2E Tests are updated and passing
  • Documentation is updated if necessary (requirement for new features)
  • Add a screenshot if the change is UX/UI related

How to test changes / Special notes to the reviewer

Add the following snippet in your app-config.yaml file

buildInfo:
  title: 'RHDH Build info'
  card:
    TechDocs builder: 'local'
    Authentication provider: 'Github'
    RBAC: disabled

@github-actions GitHub Actions
Copy link
Contributor

The image is available at:

@debsmita1
Copy link
Member Author

/retest

@openshift-ci OpenShift CI
Copy link

openshift-ci bot commented Mar 13, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign josephca for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

christoph-jerolimov
christoph-jerolimov reviewed Mar 13, 2025
View reviewed changes
Copy link
Member

@christoph-jerolimov christoph-jerolimov left a comment

Choose a reason for hiding this comment

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

Thanks for all the tests 👍

But I thought we can simple use useConfig and make the build config visibility to the frontend. Wouldn't this easier? Wdyt?

packages/app/src/build-metadata.json
"Backstage Version: 1.35.1",
"Last Commit: repo @ 2025-01-31T14:15:06Z"
]
"card": {
Copy link
Member

Choose a reason for hiding this comment

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

I think this file is generated in the build process. Maybe also in the midstream repo on GitLab.

I recommend to check this with @nickboldt, or maybe you can support an object and an array to support the old format (and revert this)?

Copy link
Member Author

Choose a reason for hiding this comment

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

There's a comment from @nickboldt here about revamping the file
I was under the impression that this file is somehow manually built

Copy link
Member

Choose a reason for hiding this comment

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

it's generated in this repo, then I apply a sed transformation downstream to spiff it up and add more build information (the up and midstream repo commits, and the build time).

      # set build-metadata.json info, using upstream info: ${ROOTPATH}/sync/upstream_SHA_rhdh-hub ==> redhat-developer/rhdh main @ 2ff35695
      sed -i packages/app/src/build-metadata.json -r \
        -e 's|"(Last Commit: )(.+)"|"Upstream: '"$upstream_repo_hub"'", "Midstream: '"$midstream_repo_and_SHA"'", "Build Time: '"$now"'"|'

-- https://gitlab.cee.redhat.com/rhidp/rhdh/-/blob/rhdh-1-rhel-9/build/ci/sync-midstream.sh#L605-607

If you change the file format, just let me know so I can adjust the downstream sed changes. Or even better, throw me a PR to update the script above to properly adopt your changes! :D

@github-actions GitHub Actions
Copy link
Contributor

The image is available at:

@debsmita1 debsmita1 force-pushed the build-info branch 2 times, most recently from 94f5296 to 6a0d499 Compare March 13, 2025 08:18
@github-actions GitHub Actions
Copy link
Contributor

The image is available at:

@debsmita1
Copy link
Member Author

/test e2e-tests

1 similar comment
@debsmita1
Copy link
Member Author

/test e2e-tests

christoph-jerolimov
christoph-jerolimov reviewed Mar 17, 2025
View reviewed changes
Copy link
Member

@christoph-jerolimov christoph-jerolimov left a comment

Choose a reason for hiding this comment

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

I think we're fine, on the other side, I feel that this code is so "complex" for what it does. But I'm almost fine when its merges.

@github-actions GitHub Actions
Copy link
Contributor

The image is available at:

@github-actions GitHub Actions
Copy link
Contributor

The image is available at:

@debsmita1 debsmita1 changed the title feat: allow overriding build info in the user settings page feat(user-settings): allow overriding build info in the user settings page Mar 19, 2025
@github-actions GitHub Actions
Copy link
Contributor

The image is available at:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nickboldt nickboldt nickboldt left review comments

@gustavolira gustavolira Awaiting requested review from gustavolira

@its-mitesh-kumar its-mitesh-kumar Awaiting requested review from its-mitesh-kumar

@invincibleJai invincibleJai Awaiting requested review from invincibleJai

@christoph-jerolimov christoph-jerolimov Awaiting requested review from christoph-jerolimov

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@debsmita1 @christoph-jerolimov @nickboldt