Feat/handle emojis GitHub #37
Conversation
|
Please remove the yarn stuff, that has nothing to do with the fix you're trying to do. If you try to add yarn please file a PR (even though I'll refuse it). In general please keep PRs focused on one thing. Thanks |
|
After you fixed the yarn thing and the issues I pointed out below, please squash all commits and push. |
package.json
Outdated
| @@ -24,5 +24,8 @@ | |||
| "gitHead": "dcde154040d17d321664a7efc38e8531330335f9", | |||
| "devDependencies": { | |||
| "tap": "~0.4.8" | |||
| }, | |||
| "dependencies": { | |||
| "emoji-regex": "^6.1.0" | |||
There was a problem hiding this comment.
Please use ~ as range specifier here.
I'm conservative with deps and also this avoids breaking older npms which might be the reason the build is breaking on travis right now.
| @@ -43,6 +43,9 @@ test('\ngenerating anchor in github mode', function (t) { | |||
| , [ 'module-specific-variables-using-jsdoc-@module', null, '#module-specific-variables-using-jsdoc-module'] | |||
| , [ 'Jack & Jill', null, '#jack--jill'] | |||
| , [ 'replace – or —', null, '#replace--or-'] | |||
| , [ 'Modules 📦', null, '#modules-'] | |||
There was a problem hiding this comment.
Is this correct to have a trailing - here?
Have you tried if github does it that way?
|
@thlorenz I'm having trouble squashing the commits since this branch has already been pushed to the PR. You should be able to squash it before you merge.
|
|
Here's a better example picture:
|
|
Sorry please squash at the command line via an interactive rebase. |
adrianmcli
force-pushed
the
feat/handle-emojis-github
branch
from
3a1c457
to
e8de22d
Compare
add tests strip emojis for github links add additional test for multiple emojis add yarn lock file remove yarn file use ~for emoji-regex dependency
adrianmcli
force-pushed
the
feat/handle-emojis-github
branch
from
e8de22d
to
c437cbd
Compare
|
There, done. |
|
Awesome, thanks now I made you collaborator. Please merge to master following this guide. Thanks a lot for your help. I'll version as a patch so doctoc will automatically pick up the fix when you install it fresh. |
|
@adrianmcli when I meant I don't want the history, I meant the commit message as well, i.e: is completely nonsensical. To me it looks like you just squashed without editing the commit message at all. I'll also publish as a patch to npm. |
|
merged and ublished as 0.5.7. |
|
Sorry about that, I thought the whole point of the squashing was to document the history of what happened without multiple commits. I'll keep this in mind in the future. |
|
The whole point of squashing is to erase the history we don't need to know, i.e. that you added a yarn file and then removed it. A commit message should state what the commit does, not how you got there, but no worries, just wanted to let you know, so you can do it better next time :) |
Fixes #36
I added 3 tests and a dependency on emoji-regex.