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

fix pyroclient #213

Merged
merged 1 commit into from
Jun 24, 2024
Merged

fix pyroclient #213

merged 1 commit into from
Jun 24, 2024

Conversation

MateoLostanlen
Copy link
Member

Following PR #204, the pyroclient commit was not fixed in src

I don't realy get the point of having two pyproject.toml @RonanMorgan @frgfm any thoughts ?

@MateoLostanlen MateoLostanlen requested review from RonanMorgan and frgfm and removed request for RonanMorgan June 24, 2024 13:40
@MateoLostanlen MateoLostanlen self-assigned this Jun 24, 2024
@MateoLostanlen MateoLostanlen added type: bug Something isn't working ext: src labels Jun 24, 2024
Copy link

codecov bot commented Jun 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.30%. Comparing base (0a1ae7c) to head (409fd1c).

Additional details and impacted files
@@           Coverage Diff            @@
##           develop     #213   +/-   ##
========================================
  Coverage    92.30%   92.30%           
========================================
  Files            6        6           
  Lines          507      507           
========================================
  Hits           468      468           
  Misses          39       39           
Flag Coverage Δ
unittests 92.30% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@frgfm
Copy link
Member

frgfm commented Jun 24, 2024

Hey there, sorry I don't get what you mean? I can only see one pyproject.toml

If you mean setup.py and pyproject.toml it's because the setuptools team and the core Python teams haven't yet agreed to a unified way of doing things. It will go away (hopefully soon)

@MateoLostanlen
Copy link
Member Author

Hi @frgfm no here we have pyproject.toml and src/pyproject.toml. Can't we unite them in one file?

@frgfm
Copy link
Member

frgfm commented Jun 24, 2024

My apologies! To be honest, I haven't contributed much to this repo in a while 😅
Back then, I think we had those separated so that the library can be used as standalone lib, but now I don't see any reason to have different pyproject and yup I agree this should be unified and similar to the API (in the sense that we need accurate version pinning, not upper/lower bound so poetry)

Happy to help if we need to make the docker image lighter & clean the build process 👌

@MateoLostanlen
Copy link
Member Author

Ok thanks for your answer. completely agree. I'll merge this pr to fix the bug then I'll make another to unify based on the pyro-api model

@MateoLostanlen MateoLostanlen merged commit 12a6a20 into develop Jun 24, 2024
14 checks passed
@MateoLostanlen MateoLostanlen deleted the fix_pyroclient branch June 24, 2024 14:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ext: src type: bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants