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.

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

Add config field for fe4.6 compatibility #373

Merged
merged 1 commit into from
Nov 21, 2024
Merged

Add config field for fe4.6 compatibility #373

merged 1 commit into from
Nov 21, 2024

Conversation

minottic
Copy link
Contributor

No description provided.

@minottic minottic requested a review from a team November 20, 2024 13:59
@minottic
Copy link
Contributor Author

depends on #374

@sbliven
Copy link
Contributor

sbliven commented Nov 21, 2024

Which commit did you take the config file from?

Would it be possible not to duplicate config.js between scicatlive and frontend? Or else use a very minimal config, where any unchanged values are omitted? Right now we frequently have big changes between config.js between frontend branches, making it hard to migrate the scicatlive configuration without causing merge conflicts. My current dev workflow is (1) build containers (2) stash changes, which are usually just config.js (3) check out the new branch, (4) stash pop.

@sbliven
Copy link
Contributor

sbliven commented Nov 21, 2024

Ahh, seems to be from the current master (last changed in 9f90abd)

Copy link
Contributor

@sbliven sbliven left a comment

Choose a reason for hiding this comment

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

I'm approving this since we need constancy with the frontend, but it would be nice to have a long-term solution.

@minottic
Copy link
Contributor Author

I inspected it in the browser and the console was giving me an error for missing defaultDatasetsListSettings, which then I realized comes from here.

I think one problem of scicatlive is that it always runs entrypoints scripts, which for the FE includes the configs merging, while it should only run them once when the container is created. I will open an issue for that

@minottic
Copy link
Contributor Author

relates to #377

@minottic minottic merged commit 29ffc0c into main Nov 21, 2024
9 checks passed
@minottic minottic deleted the feconf_4.6 branch November 21, 2024 09:05
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.

2 participants