Skip to content

IS-10950 reading listing template from env-var, previever script impr.#57

Open
markoweb wants to merge 5 commits intomainfrom
IS-10950-previewer-listing
Open

IS-10950 reading listing template from env-var, previever script impr.#57
markoweb wants to merge 5 commits intomainfrom
IS-10950-previewer-listing

Conversation

@markoweb
Copy link
Contributor

This PR is allowing to change the listing template by setting the LISTING_TEMPLATE env var.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Enables selecting the Identity Server preview “listing template” via the LISTING_TEMPLATE environment variable.

Changes:

  • Update previewer.js to append -l <LISTING_TEMPLATE> to the Java process args when the env var is set.
  • Replace string throws with Error instances for required argument validation.
  • Align package-lock.json workspaces with the current repo layout (removes non-existent src/login-web-app entry).

Reviewed changes

Copilot reviewed 1 out of 3 changed files in this pull request and generated 1 comment.

File Description
src/identity-server/previewer.js Adds env-var-driven listing template argument and improves error throwing style.
package-lock.json Updates workspace list to match the repository’s existing workspaces.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.


if (process.env.LISTING_TEMPLATE) {
procArgs.push('-l');
procArgs.push(process.env.LISTING_TEMPLATE)
Copy link

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

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

Avoid automated semicolon insertion (92% of all statements in the enclosing function have an explicit semicolon).

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@copilot open a new pull request to apply changes based on this feedback

procArgs.push(optionsOrArgv['additional-static-root'])
}

if (process.env.LISTING_TEMPLATE) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not familiar with this script, but would it make sense to pass this via optionsOrArgv, as with other things above?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That would require to change all npm scripts to allow setting this param and because it's intended just for an internal use I think the env. var. is better approach here.

Copy link

Copilot AI commented Jan 28, 2026

@markoweb I've opened a new pull request, #59, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot AI and others added 2 commits January 28, 2026 07:10
Co-authored-by: markoweb <2862929+markoweb@users.noreply.github.com>
Co-authored-by: markoweb <2862929+markoweb@users.noreply.github.com>
Add missing semicolons for ASI consistency
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.

6 participants