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

feat: use rye as project manager #2359

Merged
merged 6 commits into from
Jan 16, 2025
Merged

Conversation

cquintana92
Copy link
Collaborator

Move from poetry to rye

@acasajus acasajus enabled auto-merge (squash) January 16, 2025 11:37
@acasajus acasajus disabled auto-merge January 16, 2025 11:38
@acasajus acasajus enabled auto-merge (squash) January 16, 2025 11:56
Copy link
Contributor

@nguyenkims nguyenkims left a comment

Choose a reason for hiding this comment

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

Hey it'd be very useful if we can make sure the pinned dependencies have the same version as the poetry ones to reduce regression risk due to dependencies.

@@ -1,6 +1,12 @@
name: Test and lint
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the previous name is more explicit.

Copy link
Collaborator

@acasajus acasajus Jan 16, 2025

Choose a reason for hiding this comment

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

This name is the prefix for the jobs. Before it was shown as:
Test and lint / lint and it was confusing to see the same job twice. You'll still see lint and test in the jobs just now we'll see SimpleLogin actions / lint and SimpleLogin actions / test

- master
pull_request:
branches:
- master
Copy link
Contributor

Choose a reason for hiding this comment

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

does this mean only PR that target master will be run?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes. This way we don't duplicate jobs as we've been doing up until now.


requires-python = "~=3.10"

dependencies = [
Copy link
Contributor

Choose a reason for hiding this comment

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

is there a way to move this one at the same location as [tool.poetry.dependencies] and [tool.poetry] to faciliate git diff comparison?

Copy link
Collaborator

Choose a reason for hiding this comment

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

The issue is that poetry uses their own non-standard way of defining dependencies. Rye follows the standard convention for python dependency management with the standard format.

Copy link
Collaborator

Choose a reason for hiding this comment

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

We basically copied the poetry ones and replaced the format to the new one.

@@ -0,0 +1,389 @@
# generated by rye
Copy link
Contributor

Choose a reason for hiding this comment

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

is there a way to check if all the pinned version here match the poetry one? To minimize issues coming from dependencies.

Copy link
Collaborator

Choose a reason for hiding this comment

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

We're checking the locked version deps from poetry to rye to make sure they are the same for our deps and the transitive deps also. We'll run this in staging for a while and if looks ok for a couple days we'll run in a canary in prod also before rolling it out.

@acasajus acasajus merged commit 20056a1 into master Jan 16, 2025
3 checks passed
@acasajus acasajus deleted the feature/use-rye-as-project-manager branch January 16, 2025 13:07
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.

3 participants