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 logos_check.py to validate logos filesize/resolution #2553

Closed
wants to merge 1 commit into from

Conversation

Salamandar
Copy link
Contributor


def check(self) -> bool:
self.fails.clear()
for fn_name in dir(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

Explicitly listing the steps is clearer :)


def check_type(self) -> None:
format = self.image.format.lower()
accepted_formats = ["png", "jpeg", "webp"]
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we accept all those formats? Many software don't support web yet. And JPG doesn't allow background transparency which is nice for dark/light background. I think only PNG should be supported, but we could discuss it on the issue.

Copy link
Member

Choose a reason for hiding this comment

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

(currently we have .png exclusively in logos/ indeed)

self.fails.append(f"Image should be one of {', '.join(accepted_formats)} but is {format}")

def check_dimensions(self) -> None:
dim_min, dim_max = 96, 300
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we maybe hardcode those constants at the top of the file for easier changes?


def check_filesize(self) -> None:
filesize = self.imgpath.stat().st_size
max_size = 80_000
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we maybe hardcode this constant at the top of the file for easier changes?

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