Add compressOnly mode and also update README to discuss forks

[tunetheweb]

Fixes #53

I'm not sure how to test this to be honest as couldn't get the compression running either locally or as a GitHub action. I tested as best I could the bits without the compression bits and the code I added seemed to work, but could do with a good looking over. Or a test docker image being built if possible (sorry I'm not familiar with Docker at all!)?

Most of the changes however are to the README and the number of code changes are actually quite small.

I also moved the config load from image-processing.js to index.js as needed one bit of config in there and didn't want to load it twice.

[tunetheweb]

Oops not sure why I closed this one! Reopening...

[tunetheweb]

Gentle nudge @benschwarz !

[benschwarz]

@bazzadp, apologies for the huge delay here.

In review, I'm happy with the changes made on this, but I'd like to discuss an alternate option that I feel will improve the overall usage of image-actions.

Instead of compressOnly, I was thinking we could split out two configuration options:

• postMarkdownSummary (default to true)
• commitChanges (default to true)

What do you think?

--

Closes #50

[tunetheweb]

Instead of compressOnly, I was thinking we could split out two configuration options:

• postMarkdownSummary (default to true)
• commitChanges (default to true)

What do you think?

Hmmm... I'm not completely sure.

Why would you want to post a markdown summary to a pull_request (the only postMarkDownSummary that works) if you aren't also committing the changes to that pull_request? Wouldn’t that messaging imply the images have been compressed (which is not true if they are not committed)? And not just compressed and (effectively) uncompressed again.

And on the flip side I'm not sure why you'd want to commit changes, but not post the summary to the PR to show that the PR was changed automatically?

We always put the markdown into outputs.markdown if you want to use it separately (e.g. I'm proposing not committing for forked pull requests and instead running on master and then opening a new PR for that with the summary, for my use case for example). So the summary is always available to you whether you commit or not, so people can do extra things if they like with it, if that was your concern?

So in effect you have the data available to you to do what you like (you can even post it as a comment to the PR if you so wish in a separate manual step), and you have a commit true/false flag, do not sure as to the usefulness or intend of the change your proposed? Could you explain it a little more so I can understand fully?

[tunetheweb]

Any more thoughts here @benschwarz ?

[tunetheweb]

I saw this comment:

An option to comment-only or keep the PR from merging if an image is found larger than max width would be nice.

Is that where the desire for comment-only version comes from @benschwarz ?

That kind of makes sense, but my concern would be that you could fill up the PR with comments if you continue to push other changes to that branch that don't fix the images and run the action multiple times. Could become very messy very quickly. Updating a comment would be an alternative solution, but that would be more difficult and may not be wanted if the comment was not the latest comment. Or update if latest, and add if not latest? But then no notification for the update. All in all it seems a good bit of extra complexity so requires some extra thought so not keen to make part of this PR now.

Another alternative is to have a "fail" option on the Action and let people look at the logs for the detail if it does fail - so have the action act like a check that only optimised images are committed. That would probably be considerably less complicated, and could possibly be added to this branch if you want?

Alternatively we merge this as is and iterate.

Let me know your thoughts.

[benschwarz]

An option to comment-only or keep the PR from merging if an image is found larger than max width would be nice.

Is that where the desire for comment-only version comes from @benschwarz ?

Yep, but let's get this PR merged and tackle other ideas later.
I've posted a couple of small nitpicks, but otherwise I'm happy for this to be merged ASAP.

[tunetheweb]

Updated so think we're good to merge.

[benschwarz]

Thanks @bazzadp !