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

Update GIF encoding information #6227

Merged
merged 1 commit into from Apr 19, 2022
Merged

Update GIF encoding information #6227

merged 1 commit into from Apr 19, 2022

Conversation

raygard
Copy link
Contributor

@raygard raygard commented Apr 19, 2022

Correct Handbook documentation about encoding of GIF files from run-length to LZW.

No issue associated with this pull request.

Changes proposed in this pull request:

  • Pillow writes GIF files with LZW encoding since 8.2.0, but the Handbook was not updated to reflect this. It still refers to run-length encoding (which was not quite correct before).

Correct encoding of GIF files from run-length to LZW.
@radarhere
Copy link
Member

Just to link issues - the LZW change occurred in #5291

@raygard
Copy link
Contributor Author

raygard commented Apr 19, 2022

@radarhere I need a little help. Not sure if this is the place to ask. I haven't done a pull in a year and I'm rusty. Contribution guidelines say "When committing only documentation changes please include [ci skip] in the commit message..." to avoid CI running. Looks like tests ran; did I put it in the wrong place or misunderstand? Also, I forked, got a local repo on my machine, and forgot to make a branch and make the change there instead of in the main branch. (Not even sure if I'm using the right words there.) Can this be fixed somehow? If I nuke my fork and re-fork, will my pull get messed up or deleted? I started small w/ this doc change but am working on some GIF plugin code changes. Would like that to be done exactly right. Any advice before I screw it up? If this is the wrong venue, email me at raygard at gmail dot com.

@radarhere
Copy link
Member

Perfectly fine to ask here.

I think the simple answer is that you didn't include '[ci skip]' in the commit message. I don't see the text '[ci skip]' in your commit - https://github.com/raygard/Pillow/commit/b01a2effd2a719fb094bcef7c471b05c07c48dfa. You have included it in the PR title, but that's not the same thing.

I don't think the fact that you've made this change in your main branch is a problem necessarily, just not quite as neat as you might like.
My memory is that if you delete your fork, this pull request will actually be fine - but you don't need to go to that extreme. You could just git reset --hard HEAD^ your local main branch to bring it back in line with our main, leave the remote main branch as it is, and fork off from your local main branch.

@radarhere radarhere merged commit acc0fa0 into python-pillow:main Apr 19, 2022
@radarhere radarhere changed the title Update image-file-formats.rst [ci skip] Update GIF encoding information Apr 19, 2022
@radarhere
Copy link
Member

Or, if I just quickly merge this branch, then you can git pull python-pillow main, and fast forward to be back in line with our main.

@raygard
Copy link
Contributor Author

raygard commented Apr 19, 2022

Thanks for setting me straight on the [ci skip]. I just now used the "Fetch upstream" button on the web page for my fork and I think I'm in sync now. I had used git CLI last year but I forgot what little I knew about it then. I'm using the GitHub Desktop app in Windows for now and it's doing what I need... for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants