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

New Click Command #1114

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

adammentzer
Copy link

Added Click command to display the number of projects. Options exist to specify for projects within the bounds of the specified number of bills (inclusive) and/or less than x days old.

Copy link
Member

@Glandos Glandos left a comment

Choose a reason for hiding this comment

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

I've added some small Pythonic changes.

But globally, it would better to have a list of predicates to filter the list of project, and execute this list comprehension project = [] only once.
I bet this is possible with SQLAlchemy, but don't have the time to find it, so, it will be fine for me with Python lambdas :)

Comment on lines +134 to +137
if bills_high < bills_low:
temp = bills_high
bills_high = bills_low
bills_low = temp
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if bills_high < bills_low:
temp = bills_high
bills_high = bills_low
bills_low = temp
if bills_high < bills_low:
(bills_low, bill_high) = bills_high, bills_low

Comment on lines +143 to +144
if pr.get_bills().count() >= bills_low
and pr.get_bills().count() <= bills_high
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if pr.get_bills().count() >= bills_low
and pr.get_bills().count() <= bills_high
if bills_low <= pr.get_bills().count() <= bills_high

Comment on lines +181 to +187
click.echo(len(projects))
if emails:
list_emails = ", ".join(set([pr.contact_email for pr in projects]))
click.echo(list_emails)
if names:
proj_names = [pr.name for pr in projects]
click.echo(proj_names)
Copy link
Member

Choose a reason for hiding this comment

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

This is duplicated code from line 155-161.

@Glandos
Copy link
Member

Glandos commented Jan 28, 2023

Also, maybe a default value of 0 for bills_low should be a better default, instead of having to chose both low and high.

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.

None yet

2 participants