Merged
Conversation
MKodde
force-pushed
the
feature/sf-6-installation
branch
4 times, most recently
from
6e2bfab to
454d8e1
Compare
And make sure the console can run without major breakage
The dependency was added, config was moved. PHPUnit needed downgrade to allow CPD 6 to install
No Unit tests are part of this project (yet) if ever we want to include unit tests, we can add them at that point.
Most of the stylesheets were not included, resulting in a degraded design of the GSSP
MKodde
force-pushed
the
feature/sf-6-installation
branch
from
dc76e25 to
a6c5af3
Compare
MKodde
force-pushed
the
feature/sf-6-installation
branch
from
0921ad3 to
497968e
Compare
To demogssp.dev.openconext.local
MKodde
force-pushed
the
feature/sf-6-installation
branch
from
3c8852f to
0b33c85
Compare
And move the overwritable config to the config/openconext folder
And update the webpack config accordingly the logo is placed in an overridable /assets/openconext folder
For now, we only run the behat tests during developing a feature. When we have a functioning test env from the OpenConext-devconf we should start running these tests on that platfomr.
All of the parameters are now read from the parameters.yaml file. While at it, I also introduced the SF runtime. That was a task on the todo list
parijke
reviewed
Dec 13, 2023
Contributor
parijke
left a comment
parijke
left a comment
There was a problem hiding this comment.
In de regel prima lijkt me. Wat kleine suggesties / vragen
src/Surfnet/Gssp/Infrastructure/Controller/DefaultController.php
Outdated
Show resolved
Hide resolved
src/Surfnet/Gssp/Infrastructure/EventSubscriber/LocaleResponseListener.php
Show resolved
Hide resolved
This setting was used to kickstart older SF5 projects to let them use the new authenticator method. This is now the default. So the setting can be disabled
Member
Author
|
Thanks for the review @parijke I addressed your concerns to the best of my abilities. I did reject some as they added too little value for the additional effort. I tried to explain why in comments. Can you re-evaluate if you can approve these changes? |
parijke
approved these changes
Dec 14, 2023
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
What changed here?