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

Add minify support #1146

Merged
merged 10 commits into from
Aug 28, 2020
Merged

Add minify support #1146

merged 10 commits into from
Aug 28, 2020

Conversation

areille
Copy link
Contributor

@areille areille commented Aug 24, 2020

IMPORTANT: Please do not create a Pull Request adding a new feature without discussing it first.

The place to discuss new features is the forum: https://zola.discourse.group/
If you want to add a new feature, please open a thread there first in the feature requests section.

Sanity check:

  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Code changes

(Delete or ignore this section for documentation changes)

  • Are you doing the PR on the next branch?

If the change is a new feature or adding to/changing an existing one:

  • Have you created/updated the relevant documentation page(s)?

Closes #542

ken0x0a and others added 2 commits August 24, 2020 10:42
This feature is already exist, but not in the doc yet

Related getzola#766
@areille areille force-pushed the add-minify-support branch from 9a5aee3 to 0a8d4f3 Compare August 24, 2020 08:44
components/config/src/config/mod.rs Outdated Show resolved Hide resolved
components/site/src/lib.rs Outdated Show resolved Hide resolved
components/site/src/lib.rs Outdated Show resolved Hide resolved
@areille areille requested a review from Keats August 24, 2020 10:21
@areille areille marked this pull request as draft August 24, 2020 10:53
@areille
Copy link
Contributor Author

areille commented Aug 24, 2020

Gone to WIP status because of a problem found with minify-html.

It appears to turn :
<link href="http:&#x2F;&#x2F;areille.com&#x2F;style.css" rel="stylesheet">
into :
<link href=http:%%areille.com%style.css rel=stylesheet>

@areille areille marked this pull request as ready for review August 24, 2020 14:05
@areille
Copy link
Contributor Author

areille commented Aug 26, 2020

@Keats this is ready for review now.

Copy link
Collaborator

@Keats Keats left a comment

Choose a reason for hiding this comment

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

I think we should move the minification to write_content so we can't forget it and no need to repeat ourselves everywhere

components/site/src/lib.rs Outdated Show resolved Hide resolved
components/site/src/lib.rs Outdated Show resolved Hide resolved
components/site/src/lib.rs Outdated Show resolved Hide resolved
components/site/src/lib.rs Outdated Show resolved Hide resolved
@areille
Copy link
Contributor Author

areille commented Aug 26, 2020

I moved the minify() call inside write_content(). Hope that it is ok now !

@Keats
Copy link
Collaborator

Keats commented Aug 26, 2020

Seems good, can you add a couple of tests for the minified output?

@areille
Copy link
Contributor Author

areille commented Aug 26, 2020

I added a test for index.html. Please let me know if this is not enough and I will add some tests for other files.

EDIT : it appears to break other tests, will correct that

@areille areille requested a review from Keats August 26, 2020 20:41
@Keats
Copy link
Collaborator

Keats commented Aug 28, 2020

Thanks! I'll bump the pinned version myself for the build to pass

@Keats Keats merged commit 0df3631 into getzola:next Aug 28, 2020
@areille areille mentioned this pull request Sep 6, 2020
3 tasks
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.

3 participants