-
Notifications
You must be signed in to change notification settings - Fork 63
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
GPII-2653: Initial onboarding of the PSP - starting with just the language setting #606
base: master
Are you sure you want to change the base?
Conversation
CI job passed: https://ci.gpii.net/job/universal-tests/825/ |
ok to test |
CI job failed: https://ci.gpii.net/job/universal-tests/880/ |
@amb26 I think you just need to merge scripts/vagrantCloudBasedContainers.sh because the repository name for the preferences loader might have changed. Otherwise the build looks okay. |
CI job failed: https://ci.gpii.net/job/universal-tests/885/ |
Failed due to https://issues.gpii.net/browse/GPII-3094, trying again |
ok to test |
CI job passed: https://ci.gpii.net/job/universal-tests/887/ |
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.
Beyond the two comments I just provided, this is looking great.
In addition to this, we need to add:
- Tests
- A description of the solution
- An np set that one can use to test this functionality
@@ -25,6 +25,9 @@ | |||
"http://registry.gpii.net/common/highContrast/enabled": true, | |||
"http://registry.gpii.net/common/highContrastTheme": "white-black", | |||
"http://registry.gpii.net/common/supportTool": ["dictionary"] | |||
}, | |||
"http://registry.gpii.net/applications/net.gpii.psp": { | |||
"http://registry.gpii.net/common/language": "en" |
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.
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.
Yep, I think it should
"path": "net.gpii.psp" | ||
}, | ||
"supportedSettings": { | ||
"language": {}, |
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.
Don't we need to include the metadata for this settitng? I guess that we want it to be there if we want to get it rendered in the PSP.
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.
Also in order to support editing it in the PPT
No description provided.