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

Feat: bring up minifyUrls #98

Merged
merged 2 commits into from
Oct 11, 2020
Merged

Conversation

SukkaW
Copy link
Contributor

@SukkaW SukkaW commented Oct 10, 2020

The PR closes #10.

The document will be updated in another PR after this one is merged.

P.S.

Let me know when the docs are ready then I'll release a new version of htmlnano. #95 (comment)

I'm also refactoring Object.keys into Object.entries which will land in another PR. That PR should be included in the new release as well.

@@ -27,4 +27,5 @@ export default {
removeRedundantAttributes: false,
removeComments: 'safe',
sortAttributesWithLists: true,
minifyUrls: false,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

minifyUrls should be considered as dangerous and shouldn't be enabled by default.

The option is not added in the max preset either since it requires the user's configuration.

@maltsev
Copy link
Member

maltsev commented Oct 11, 2020

@SukkaW thanks a lot for these PRs! I highly appreciate your contributions to htmlnano!

@maltsev
Copy link
Member

maltsev commented Oct 11, 2020

The document will be updated in another PR after this one is merged.

Thanks! Though, is there a reason to not include the docs updates in the same PR?

@maltsev
Copy link
Member

maltsev commented Oct 11, 2020

I'm also refactoring Object.keys into Object.entries which will land in another PR. That PR should be included in the new release as well.

Sure. Thanks! Then I'll wait with releasing a new version until that PR is merged as well.

@SukkaW
Copy link
Contributor Author

SukkaW commented Oct 11, 2020

Thanks! Though, is there a reason to not include the docs updates in the same PR?

I'm not sure if my implementation will be accepted. There might also be a better implementation for the same feature, so I decide to split the document into another PR.

I can include both implementation and the document change in one PR if you wish.

@maltsev
Copy link
Member

maltsev commented Oct 11, 2020

I'm not sure if my implementation will be accepted. There might also be a better implementation for the same feature, so I decide to split the document into another PR.

Fair enough. Let's keep it that way then.

@SukkaW SukkaW mentioned this pull request Oct 11, 2020
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.

Minify URLs
2 participants