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

Misc Cleanup of documentation and Build #115

Closed
wants to merge 5 commits into from
Closed

Conversation

gam3
Copy link
Contributor

@gam3 gam3 commented Aug 16, 2016

I did some misc cleanup of the documentation. Fixed the README file so that it works for YARD and Github (there must be a better way though). And updated the minitest class names.

 - CHANGED: made rubocop optional
 - CHANGED: have links in markdown files work for Github and YARD
 - CHANGED: Minitest::Unit::TestCase -> Minitest::Test
 - FIXED: Typo in docs for Domain#new
 - FIXED: dead publicsuffix/format link
@weppos
Copy link
Owner

weppos commented Aug 16, 2016

Hi @gam3, I appreciate the cleanup. However, the patch contains several changes, and I'm not inclined to merge some of them.

Specifically, I don't want to monkey path Yard to support Markdown. Moreover, I'd avoid changing the gemspec.

Can you please provide a patch with only the documentation updates?

Also, I don't think I agree to make Rubocop optional. What is the reason about it?

@gam3
Copy link
Contributor Author

gam3 commented Aug 17, 2016

I will break this into a few separate pull requests.
You might want to apply the 1f1b03b change yourself as it is a bug.

I made Rubocop optional because it really only needs to be run by developers. There is no reason to require the rubocop gem to be installed just to install the publicsuffix gem.

@weppos
Copy link
Owner

weppos commented Oct 15, 2016

I merged 1f1b03b

@weppos weppos closed this Oct 15, 2016
@weppos weppos self-assigned this Oct 15, 2016
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

2 participants