Skip to content
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

These are reftests of vector effects of svg 2 other than non scaling stroke. #13302

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

satakagi
Copy link

@satakagi satakagi commented Oct 2, 2018

This is a pull request that solved issues pointed out on #12712

Copy link
Contributor

@svgeesus svgeesus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apart from the flags and assertion comments (which apply to all the tests) these look good to me.

<html:link rel="help"
href="https://svgwg.org/svg2-draft/coords.html#VectorEffects"/>
<html:link rel="match" href="ve-fixedPosition1-ref.svg" />
<metadata class="flags">TOKENS</metadata>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<metadata class="flags">TOKENS</metadata>

Either use specific tokens for the flags or, if none are needed, remove the metadata element

href="https://svgwg.org/svg2-draft/coords.html#VectorEffects"/>
<html:link rel="match" href="ve-fixedPosition1-ref.svg" />
<metadata class="flags">TOKENS</metadata>
<desc class="assert">TEST ASSERTION</desc>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly, either remove the assertion or (better) describe in prose what specifically each test is testing. In this case the title is pretty descriptive so I would just remove the assertion.

@AmeliaBR
Copy link
Contributor

@satakagi Are you able to make the edits requested (here and in #12712), or would you like someone else to take over?

@svgeesus svgeesus requested review from svgeesus and removed request for svgeesus December 13, 2021 19:35
@svgeesus svgeesus self-requested a review June 29, 2022 08:07
@svgeesus svgeesus enabled auto-merge (rebase) June 29, 2022 08:11
@svgeesus svgeesus assigned caribouW3 and unassigned svgeesus Jan 6, 2023
@svgeesus svgeesus requested a review from caribouW3 January 6, 2023 12:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants