doc/readme migration progress update #428
Conversation
|
|
||
| Important: When completing a frontend migration, please update the below list regarding the component you have migrated. | ||
|
|
||
| ### SUMMARY: |
There was a problem hiding this comment.
The SUMMARY has two “components migrated” counts. Could we label each count with the branch/context (e.g., “9.x:” vs “10.0.x:”) so readers don’t have to infer what 89/183 vs 125/185 refers to?
There was a problem hiding this comment.
Hi @rubalpreet-singh-123, I'm unable to replicate the two counts you mentioned. Could you have been reviewing the file in 'diff' mode, displaying the previous code and current in the same view? This is what displays for me rendered.
and view as diff:
rubalpreet-singh-123
left a comment
rubalpreet-singh-123
left a comment
There was a problem hiding this comment.
Reviewed the README migration progress update. Left a suggestion to clarify the summary counts so readers can better understand the context.
README.md
Outdated
| - [x] ./src/app/common/alert-list/alert-list.coffee | ||
| - [x] ./src/app/common/modals/progress-modal/progress-modal.coffee | ||
| - [x] ./src/app/errors/states/not-found/not-found.coffee | ||
| - [x] ./src/app/groups/tutor-group-manager/tutor-group-manager.coffee |
There was a problem hiding this comment.
This component seems to be duplicated in same section. Could you take a look at this and remove one of them for consistency?
There was a problem hiding this comment.
@mannat2634 Great catch thanks! I've removed the duplicate.
README.md
Outdated
| - [ ] ./src/app/visualisations/alignment-bullet-chart.coffee (ILO Alignments removed) | ||
| - [x] ./src/app/groups/group-member-contribution-assigner/group-member-contribution-assigner.coffee | ||
| - [x] ./src/app/groups/groups.coffee | ||
| - [x] ./src/app/projects/states/dashboard/directives/progress-dashboard/progress-dashboard.coffee (9.x) |
There was a problem hiding this comment.
For better consistency, should we standardize the use of branch/version tags for components that have been fully migrated? Like we have some clear tags (Removed in 10.0.x) but some ambigious (9.x). Like, what does the version name mean? We see (IN 10.0.x) and (9.x) used, but no clarification on what the absence of a tag implies (e.g., is it a new, non-migrated file, or a fully refactored one?).
Since this list is primarily about migration status, a clearer, consistent rule for these inline notes would be helpful for future updates.
There was a problem hiding this comment.
@mannat2634 Thanks that makes sense, I've removed the 9.x reference as suggested to avoid the ambiguity.
There was a problem hiding this comment.
@mannat2634 Thanks that makes sense, I've removed the 9.x reference as suggested to avoid the ambiguity.
Thanks for quick fixes. Changes seem fine to me. I don't find any further comments/improvements on this.
Thank you!
|
Hi, I reviewed the changes and update seems to be clear and accurate for the most part. I had some suggestions which have been added as comments, mainly to improve the clarity of tag meanings. |
|
approved for upstream, @kacieje you can now open a PR to upsteam repo |
4 participants
Description
Migration progress has been updated to reflect the current state of the 9.x branch. Files that have been flagged as not used/required within 10.0.x have been separated to make clear that removal has not yet occurred. i.e. the files are still present in both 9.x and 10.0.x as at the time of this PR.
Type of change
How Has This Been Tested?
The updated content has been tested against the thoth-tech/doubtfire-web: 9.x, thoth-tech/doubtfire-web: 10.0.x, doubtfire-lms/doubtfire-web: 9.x and doubtfire-lms/doubtfire-web: 10.0.x branches. The migration_progress script output was also compared however the output results cannot be simply copied across as it does not account for those files completed within 10.0.x.
The actual changes of this PR have been tested through the file previews to ensure the display aligns with existing formatting structures.
Testing Checklist:
Checklist:
Files Modified