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

clarify what input the package handles (issue #30) #31

Merged
merged 2 commits into from
Jun 25, 2017
Merged

clarify what input the package handles (issue #30) #31

merged 2 commits into from
Jun 25, 2017

Conversation

waldyrious
Copy link
Contributor

@alexanderwallin
Copy link

👌

Perhaps a link to https://developer.mozilla.org/en-US/docs/Web/SVG/Tutorial/Paths or some other source?

README.md Outdated
@@ -6,11 +6,12 @@ svgpath
[![Coverage Status](https://img.shields.io/coveralls/fontello/svgpath/master.svg?style=flat)](https://coveralls.io/r/fontello/svgpath?branch=master)


> Low level toolkit for SVG paths transformations. Sometime you can't use
`transform` attributes and have to apply changes to svg paths directly.
Low level toolkit for SVG paths transformations.
Copy link
Member

Choose a reason for hiding this comment

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

Please keep formatting. Return > to start of line

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, yes, I forgot to mention that. With the >, that description becomes greyed out and it's a little hard to read (especially for what's supposed to be the main description of the repo). Perhaps some different formatting could be used, like italics?

Copy link
Member

Choose a reason for hiding this comment

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

No. Please, keep old style.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, sure.

@waldyrious
Copy link
Contributor Author

@alexanderwallin good point. Linking to https://www.w3.org/TR/SVG11/paths.html#PathData may be more appropriate, agreed?

@puzrin
Copy link
Member

puzrin commented Jun 23, 2017

No need to make things complex. Just modify example to use real string and show result.

@alexanderwallin
Copy link

I would appreciate that link.

@waldyrious
Copy link
Contributor Author

I've restored the blockquote formatting, as requested. I also added the link to the docs -- I agree with @alexanderwallin that it is useful to end-users.

@puzrin
Copy link
Member

puzrin commented Jun 25, 2017

Ok. Please, join all to single comit and i will merge it

@waldyrious
Copy link
Contributor Author

That can be done directly on github now :) just use the "Squash and merge" option, like this:

screenshot from 2017-06-25 16-31-33

@puzrin
Copy link
Member

puzrin commented Jun 25, 2017

That leave garbage in comment lines. Could you join manually and force push? Text - something like "Clarify description in readme"

@waldyrious
Copy link
Contributor Author

Oh, so you tried it before? You might want to check it out again -- the functionality probably has been changed since you last used it, because now it does allow editing the full commit message (including the extended description) before merging, directly from the web interface. It's quite convenient, really :) But I can do it myself in the command line if you prefer.

@puzrin puzrin merged commit c8a2840 into fontello:master Jun 25, 2017
@puzrin
Copy link
Member

puzrin commented Jun 25, 2017

Seems to work. Thank you for info, now web squash is really useable.

@waldyrious
Copy link
Contributor Author

Yeah, they've been making several nice improvements to the UI recently, which makes both contribution and maintenance easier :)

@waldyrious waldyrious deleted the patch-1 branch June 25, 2017 20:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants