-
Notifications
You must be signed in to change notification settings - Fork 132
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
Accessibility assessment - gc-featured-link and PFT #2256
Conversation
57fe2ac
to
f3d688e
Compare
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.
See the inline change request and you will need to remove the merge commit.
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.
Quick note: There is a merge commit that create conflict and would need to be manually fixed.
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.
We will need to remove/replace those merge commit by a normal commit. It do create conflict when I am trying to git rebase upstream/master
. The easiest I think will be to create a new fresh commit by using the github diff.
I am going to do a functional review but it looks good from a content point of view.
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.
Tested from a functional content point of view and it is all good and do work as expected.
One little change left and to remove the merge commit.
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.
A few editing fix required. You did fixed them but they came back during our rebase/squash process.
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.
Reviewed and tested locally
It do work as expected.
I am going to add the link to the report in another PR along with a content sync.
No description provided.