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

Don't generate .png typegraph files #1131

Merged
merged 1 commit into from Jan 31, 2017
Merged

Don't generate .png typegraph files #1131

merged 1 commit into from Jan 31, 2017

Conversation

coke
Copy link
Collaborator

@coke coke commented Jan 11, 2017

For consideration.

Untested, as 'make htmlify' is insanely slow on my particular machine at the moment. Should resolve #1063 if it's accepted.

@coke
Copy link
Collaborator Author

coke commented Jan 19, 2017

I'll go ahead and merge this in a week with no objection.

@coke coke self-assigned this Jan 19, 2017
@AlexDaniel
Copy link
Member

AlexDaniel commented Jan 19, 2017

Code removed is code that is doesn't have to be maintained.

So the pull request removes 14 lines of code and one feature that could be useful to some. The justification is that it is slow on your machine.

Well, then just implement --light option that makes it not generate png graphs, or maybe even all graphs, so that it is faster on your machine. This would be useful for other purposes too.

👎

@coke
Copy link
Collaborator Author

coke commented Jan 19, 2017 via email

@coke
Copy link
Collaborator Author

coke commented Jan 21, 2017

BTW, the justification is not only that "it is slow on my machine" (the doc build is VERY slow, and this is a drop in the bucket). It's extra time spent on every build, and we don't need two ways to do something when one suffices. Any extra code you have is code that has to be maintained.

http://caniuse.com/#feat=svg has a listing of all the browsers that support SVG.

I intended the discussion to be centered around whether or not that browser listing is recent enough to drop the PNGs, my assertion being that it is.

Also, Adding another switch is another thing that has to be maintained.

@coke
Copy link
Collaborator Author

coke commented Jan 23, 2017

Feedback on https://irclog.perlgeek.de/perl6/2017-01-23#i_13975600 seems to generally be: these were useful once but are no longer.

@zoffixznet
Copy link
Contributor

So the pull request removes [...] feature that could be useful to some [...] just implement --light option

I see no reason to add, document, and support an option on a sole basis of "could be useful to some." Especially considering the ease of converting the SVGs to PNGs should anyone actually need the latter (convert does a good job even without any additional settings).

we don't need two ways to do something

Or to waste bandwidth on bots crawling those PNGs (yes, I heard of robots.txt)

whether or not that browser listing is recent enough

It is. SVGs been supported for ages. The only browsers I recall giving trouble are earlier IEs that are no longer even supported by Microsoft in any way. To corroborate, here's a SO question from 5 years ago with the answer that all browsers supported it for years.

I'll go ahead and merge this in a week with no objection.

👍

@coke
Copy link
Collaborator Author

coke commented Jan 31, 2017

Based on IRC feedback and zoffix's last comment, merging.

@coke coke merged commit 525e94e into master Jan 31, 2017
@coke coke deleted the coke/pngless branch January 31, 2017 00:47
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.

consider not generating PNGs.
3 participants