Update visibilities properties and constructor in Dashboard
A partial approach on refactoring the Dashboard (see https://github.com/ILIAS-eLearning/ILIAS/pull/6539):
- add
finalandreadonly - update constructors
- removed trivial local variables
@alex40724 @klees The refactoring also contains some minor changes within the depending classes in RecommendedContent, StudyProgramme and LearningSequence. This is just cosmetic/conventional nature and does not interefere with any process. Pls have a short look.
Greetings, @iszmais
@alex40724 @klees Just a ping i case this here was overlooked.
The taken changes in your components are quite small:
- changes property from
viewSettingstoview_settings - changed function name from
getCurrentViewtogetView(since there is only 1 view) - applied 2 final states to function
Therefore i assume there is not much to review at all, but i would reluctantly merge this without your confirmation in respect to your maintainance/authority.
Greetings, @iszmais
:bell:
@klees @alex40724 :bell:
@klees Thank you for your feedback.
I used the final keyword to "guide" future progress on these providers by promoting a singular (or dual?) structure in this provider system. This should prevent the creation of inheritance clusters and intransparent structures as well as component internal duplication, especially on "critical" functionality like management operations or general identification.
This should further not impede the testing, but, on the contrary, help it by structuring the conceptional requirements on this provider system within the code basis. This way we could prevent numb testing and make sure the test themself doesn't encourage behavior that contradicts the general concept.
However, I'm aware that the final keyword only tackles this issue partially and works very one-sided.
Due to that, it's no requirement to me either, just a suggestion.
Therefore, I removed it from your supervised components.
Greetings, @iszmais
Hi @iszmais,
the Technical Board looks into oldish PRs regularly, this is when this PR came up. Is there anything that we can do here to move this to some definite state, be it a merge or close?
Kind regards!
@alex40724 / @smeyer-ilias All eyes on you. :smile:
As soon as you can give your approval to the changes in Repository (Recommended Content), this will be ready to merge.
Greetings, @iszmais
@alex40724 / @smeyer-ilias :bell:
@alex40724 / @smeyer-ilias :bell: