-
Notifications
You must be signed in to change notification settings - Fork 72
docs: add projects/index migration investigation and plan #165
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
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for thoth-tech ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
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.
Hi @JeffySam,
I checked out your changes locally. The content of the two documents seems a little trivial, but I think there's probably a place for this kind of process or action log. I don't think that place is in the root Markdown directory of src/content/docs; currently the only way to view your contribution is to directly navigate to /projects_index_migration_plan/, try this link to see what I'm talking about - or similar for your investigation document.
- Please try to find a new location for these two files, perhaps under
src/content/docs/Products/OnTrack/Documentation/Front End Migration/Migration.
In addition, the table on the rendered page looks kind of bad:
Notice the right-hand column extending all the way to the right of the container. This isn't your fault, but I think it should be fixed:
- You can find the custom CSS at
src/content/styles/custom.css. Please add the following property and value inside the CSS rule for.sl-markdown-content tableselector:max-width: max-content;
|
hi @SteveDala Thanks for the review!
|
SteveDala
left a comment
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 are still a couple of nits. See below
| verify that subscriptions are cleaned up correctly, and confirm that behaviour | ||
| matches the legacy implementation. | ||
|
|
||
| ## Child State Verification Matrix |
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.
The table that was previously here no longer exists. Was this intentional? If so, please remove this heading.
|
|
||
| ## Child State Verification Matrix | ||
|
|
||
| ## Child State Verification Checklist |
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.
If this is a checklist now, it would be better to fill in the checks with [x] so that it's clear these were checked as part of your process.
|
@SteveDala Thanks for the follow-up!
|
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.
Approved. @BrianDangDev please proceed with an approving review to merge.
|
Hi @SteveDala , quick heads-up — I removed the Units module docs from this PR as they’re meant for a separate documentation PR. The originally approved content is unchanged. Could you please re-approve when you get a moment? Thanks! |
|
As the content is not substantially different than my previous approval, the approval stands. |
Description
This PR adds documentation for the migration of the
projects/indexparent state from AngularJS to Angular.Included documents
Projects Index Migration Investigation
Projects Index Migration Plan
Scope
Purpose
To provide clear technical context and a structured, low-risk path
to finalising the
projects/indexparent state migration.