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

Nitpicks #942

Merged
merged 3 commits into from
Oct 7, 2014
Merged

Nitpicks #942

merged 3 commits into from
Oct 7, 2014

Conversation

manisandro
Copy link
Member

Just some trivial fixes for things rpmlint complains about:

  • Jpeg2KEncode.c should not be executable
  • OleFileIO.py should not have a shebang since it is not an executable python script
  • WalImageFile.py should be utf-8 encoded

@hugovk
Copy link
Member

hugovk commented Oct 3, 2014

OleFileIO.py has an if __name__ == "__main__": section so is executable as a standalone script, even though Pillow doesn't use it that way.

Also, I'm not sure if we're taking changes to OleFileIO.py as it's also maintained elsewhere, but having said that we have made some changes recently which should probably be contributed back upstream, so I'll leave this to the other project members to decide. This also concerns PR #931.

@manisandro
Copy link
Member Author

Oh yes I missed the if __name__ == "__main__":. In that case, we could just make the file executable (and fix the shebang along the way to use /usr/bin/env python like the other scripts).

@hugovk
Copy link
Member

hugovk commented Oct 7, 2014

@wiredfool @aclark4life Are we ok to take changes for OleFileIO.py? See also #931.

@aclark4life
Copy link
Member

+0, although if it's just nitpicks with no serious consequences then +1 for sure.

@wiredfool
Copy link
Member

I don't think the changes to OleFileIO are going to lead to messy merges with Philippe's future changes. So, fine with me.

hugovk added a commit that referenced this pull request Oct 7, 2014
Fixes for things rpmlint complains about
@hugovk hugovk merged commit 9486390 into python-pillow:master Oct 7, 2014
@hugovk
Copy link
Member

hugovk commented Oct 7, 2014

Thanks!

@hugovk hugovk mentioned this pull request Oct 7, 2014
@manisandro manisandro deleted the nitpicks branch October 7, 2014 18:42
@decalage2
Copy link

Hi, I am not sure why the encoding declaration in OleFileIO is an issue, but removing it should be fine. However, according to PEP 0263, the absence of encoding declaration means the encoding is "ascii" (7-bits), and not UTF-8 as mentioned in commit 0ca102f. If UTF-8 is preferred, I think it should be explicit with an encoding declaration.
I will merge these changes (shebang + coding) into the new version of olefile that I am preparing, then I will submit a pull request.

@manisandro
Copy link
Member Author

My bad, thanks for spotting.

@hugovk
Copy link
Member

hugovk commented Oct 8, 2014

Thanks @decalage2!

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.

5 participants