Skip to content

Conversation

@provokateurin
Copy link
Member

Trying to understand the Makefile with all these unnecessary variables is really hard, so I just removed them and moved almost everything inline.

@provokateurin provokateurin requested a review from bigcat88 April 29, 2024 07:37
Copy link
Member

@joshtrichards joshtrichards left a comment

Choose a reason for hiding this comment

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

Made one small additional adjustment that was needed. Passes all tests now.

@provokateurin
Copy link
Member Author

Maybe @bigcat88 you can review this as well?

@bigcat88
Copy link
Member

Made one small additional adjustment that was needed. Passes all tests now.

Thank you again

Maybe @bigcat88 you can review this as well?

Looks good, let's merge this

coverage=$(poetry_run) coverage
manage-script=$(CURDIR)/manage.py
manage=$(poetry_run) $(manage-script)
db=sqlite
Copy link
Member

@bigcat88 bigcat88 Jun 10, 2024

Choose a reason for hiding this comment

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

please return "$db" variable, it is used in tests. except these all other changes are good

Signed-off-by: Alexander Piskun <[email protected]>
Copy link
Member

@bigcat88 bigcat88 left a comment

Choose a reason for hiding this comment

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

now the tests for pgsql is run on pgsql and not on sqlite

@bigcat88 bigcat88 merged commit 8cab226 into master Jun 10, 2024
@bigcat88 bigcat88 deleted the refactor/makefile/cleanup branch June 10, 2024 14:27
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.

4 participants