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 fake setup.py #2351

Merged
merged 7 commits into from
Sep 1, 2022
Merged

Add fake setup.py #2351

merged 7 commits into from
Sep 1, 2022

Conversation

Kludex
Copy link
Sponsor Member

@Kludex Kludex commented Aug 26, 2022

This is quite sad.

@michaeloliverx brought this to our attention on: 45b7cfa#commitcomment-82042745.

The Used by section on the project's profile disappeared after we removed the setup.py. We can either accept this PR or revert #2334, to have it back,

I'll let you folks decide @florimondmanca @tomchristie 🙏

I'll update the Starlette and Uvicorn according to what is decided here.

Closes #2352

@michaeloliverx
Copy link
Member

michaeloliverx commented Aug 26, 2022

This is the same solution as urllib3 has taken. You can read a bit more about it at https://sethmlarson.dev/blog/preparing-for-the-wave-of-open-source-funding#dependency-chain-metadata.

Edit:

Probably good to link the underlying problem. https://github.com/orgs/community/discussions/6456

@Kludex
Copy link
Sponsor Member Author

Kludex commented Aug 26, 2022

@florimondmanca
Copy link
Member

That’s a raging GitHub limitation. I’d be okay with this PR; happier to use newest standards than stick to oldest because GH is lagging behind. The sad part is we’d have to do this and duplicate dependency declaration every project. So I’m on the fence…

@Kludex
Copy link
Sponsor Member Author

Kludex commented Aug 26, 2022

How we decide here?

@Kludex
Copy link
Sponsor Member Author

Kludex commented Aug 28, 2022

I've created #2352 as an alternative to this PR.

@tomchristie
Copy link
Member

tomchristie commented Aug 30, 2022

"Flask does this" is a pretty good argument. 😅

The sad part is we’d have to do this and duplicate dependency declaration every project. So I’m on the fence…

Indeed. I'm not wild about it either, but the pallets team have this on all their repos. Probably an okay solution for now.

setup.py Show resolved Hide resolved
setup.py Show resolved Hide resolved
@Kludex
Copy link
Sponsor Member Author

Kludex commented Aug 31, 2022

"Flask does this" is a pretty good argument. 😅

The sad part is we’d have to do this and duplicate dependency declaration every project. So I’m on the fence…

Indeed. I'm not wild about it either, but the pallets team have this on all their repos. Probably an okay solution for now.

Since that's the case. Shall we choose this PR?

@Kludex Kludex requested a review from a team August 31, 2022 05:46
setup.py Show resolved Hide resolved
@Kludex
Copy link
Sponsor Member Author

Kludex commented Sep 1, 2022

I'll merge this tonight if no objections are made. 🙏

Thanks @michaeloliverx 🙏

@Kludex Kludex merged commit 9f79b5c into master Sep 1, 2022
@Kludex Kludex deleted the fix-used-by-github branch September 1, 2022 16:13
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

5 participants