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

Drop sanitized_anchor_name dependency #139

Closed
igungor opened this issue Jan 5, 2015 · 6 comments · Fixed by #140
Closed

Drop sanitized_anchor_name dependency #139

igungor opened this issue Jan 5, 2015 · 6 comments · Fixed by #140
Assignees

Comments

@igungor
Copy link

igungor commented Jan 5, 2015

Pulling russross/blackfriday brings the huge github.com/shurcooL/go/ repo because this package needs a simple sanitized_anchor_name.Create function. It can be written easily. Please consider sticking to the standard library if possible. Thanks.

@dmitshur
Copy link
Collaborator

dmitshur commented Jan 5, 2015

I agree that pulling in that large of a repository for this small need is not very justifiable.

I see some possible alternatives:

  1. Moving the sanitized_anchor_name package out into a small separate git repository.
  2. Moving the sanitized_anchor_name package into blackfriday repository itself (i.e., github.com/russross/blackfriday/sanitized_anchor_name).
  3. Simply copying the code of sanitized_anchor_name.Create into private unexported code of blackfriday and using it internally.
    • With this approach, other packages that want to use the same logic to create identical sanitized anchor names (compatible with what blackfriday generates) will not be able to reuse the logic. See here for current list of importers of sanitized_anchor_name.

It seems that the sanitized_anchor_name is now pretty feature-complete and stable, and I don't anticipate any changes/improvements to it in the future. But I can't really predict that with full certainty.

@igungor
Copy link
Author

igungor commented Jan 5, 2015

both 2 and 3 sounds good.

@dmitshur
Copy link
Collaborator

dmitshur commented Jan 7, 2015

Option 3 is not good because other packages import and rely on same logic, and they wouldn't have something to import if it's done.

@rtfb
Copy link
Collaborator

rtfb commented Jan 7, 2015

@shurcooL, since sanitized_anchor_name is your code, would you prefer option 1 or option 2? I think it's essentially your call.

@dmitshur
Copy link
Collaborator

dmitshur commented Jan 8, 2015

I think let's go with option 1. Otherwise, if another package needs this logic, someone else can make the same argument:

Pulling xyz brings the github.com/russross/blackfriday repo because this package needs a simple sanitized_anchor_name.Create function.

I can move it to github.com/shurcooL/sanitized_anchor_name or under another github username/organization, I don't mind.

If that sounds good to everyone, I'll make the move and create a PR that closes this issue.

@rtfb
Copy link
Collaborator

rtfb commented Jan 8, 2015

shurcooL/sanitized_anchor_name sounds perfectly fine to me.

@dmitshur dmitshur self-assigned this Jan 8, 2015
dmitshur added a commit that referenced this issue Jan 11, 2015
It has moved into a smaller standalone repo.
Closes #139.
@rtfb rtfb closed this as completed in #140 Jan 12, 2015
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 a pull request may close this issue.

3 participants