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 support for Lilya #977

Merged
merged 5 commits into from
Apr 6, 2024
Merged

Conversation

tarsil
Copy link
Contributor

@tarsil tarsil commented Apr 5, 2024

Adding extra support for Lilya.

@codecov-commenter
Copy link

codecov-commenter commented Apr 5, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.78%. Comparing base (363d683) to head (5bbbf63).

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #977   +/-   ##
=======================================
  Coverage   92.78%   92.78%           
=======================================
  Files         108      108           
  Lines        8182     8182           
=======================================
  Hits         7592     7592           
  Misses        590      590           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@tarsil
Copy link
Contributor Author

tarsil commented Apr 5, 2024

@dantownsend not sure how to address this linting situation? Because its complaining about files that I haven't touched?

template = Environment(
loader=FileSystemLoader(searchpath=dir_path)
).get_template(file_name)
template = Environment(loader=FileSystemLoader(searchpath=dir_path)).get_template(
Copy link
Member

Choose a reason for hiding this comment

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

@tarsil Linter complains because the line is too long (here and in few other places). Piccolo uses 79 as the line length. Here are the code style docs.
First you need to run linters and formaters locally with this set of commands:

  1. Run formaters: ./scripts/format.sh
  2. Run linters: ./scripts/lint.sh

That should fix the linting error. Hope that helps.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sinisaos perfect. Forgot the format.sh. Done it now.

Appreciated your help on this and now the CI should work fine.

@tarsil
Copy link
Contributor Author

tarsil commented Apr 5, 2024

@sinisaos now this is odd. I ran exactly those commands and have no problems locally and still complains. Any thoughts?

EDIT: I installed again the requirements and run the scripts.

@dantownsend
Copy link
Member

This looks good to me, but I'll let @sinisaos have the final say.

@sinisaos
Copy link
Member

sinisaos commented Apr 5, 2024

@dantownsend @tarsil It seems good to me too, but I would still try it with piccolo asgi new to see if everything is ok. Unfortunately I can't try it until later tonight. I'll report back here after I try it out.

@dantownsend
Copy link
Member

@sinisaos Thanks

@tarsil
Copy link
Contributor Author

tarsil commented Apr 5, 2024

Thank you @sinisaos

@sinisaos
Copy link
Member

sinisaos commented Apr 6, 2024

@dantownsend This works great. @tarsil thanks for your work. Can you add Lilya to readme.md as well. Thanks again.

@tarsil
Copy link
Contributor Author

tarsil commented Apr 6, 2024

@dantownsend This works great. @tarsil thanks for your work. Can you add Lilya to readme.md as well. Thanks again.

Thank you 🙂. Done.

@dantownsend dantownsend merged commit 919983a into piccolo-orm:master Apr 6, 2024
46 checks passed
@dantownsend
Copy link
Member

Cool, thanks both.

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

4 participants