feat: split up management file #592
Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.OpenSSF Scorecard
Scanned Manifest Files |
|
There was a problem hiding this comment.
I think most of the functions defined inside the listener of DOMContentLoaded can be moved outside the listener, the listener should be only for calling the functions
There was a problem hiding this comment.
Not an AC, will make a separate task for these changes.
There was a problem hiding this comment.
The purpose of the split is to increase the clarity and maintainability of the files of the management UI, copy-pasting the code inside of the management UI leave us in more or less the same situation as if the split never happened. Therefore, it makes sense that this change should be part of this task and not of a separated one.
There was a problem hiding this comment.
We should not modify AC of a task after the PR is already out in review.
There was a problem hiding this comment.
I recommend to try to minimize the access to data object from inside the functions, it should be received as a parameter instead so we don't use it as a global variable, that will make the function more readable and less error prone
There was a problem hiding this comment.
Not an AC, will make a separate task for these changes.
There was a problem hiding this comment.
The purpose of the split is to increase the clarity and maintainability of the files of the management UI. Using a global variable inside most of the management UI functions is the oposite from the purpose of the task, as it increases the chances of generating side-effects on the call to those functions, so in that case we would degrade the code quality rather than increasing it, as it was originally intended.
There was a problem hiding this comment.
Purpose of this task was to split up the management.html and getConfig function. Code refactorings can be done in a separate task where it wont impact visibility of this PR's code changes. If I add refactoring here it will be difficult to see new code from code that has been moved during split.
What:
Split up management.html file into multiple file
Refactored getConfig
Task:
GBI-2370