-
Notifications
You must be signed in to change notification settings - Fork 357
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 rye as a package manager. #46
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One minor comment otherwise great stuff ⛩️ 💪
CONTRIBUTION.md
Outdated
3. Add Rye on your system if it is not installed already. [Source](https://rye-up.com/guide/installation/) | ||
4. Install all the dependencies using `rye sync`. | ||
5. Install pre-commit hook using `rye run pre-commit install` | ||
6. Run `rye add package_name` (if you wish to add any packages as per your requirements.) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would also be good to hint that sync is necessary, as add
does not provide the packages to the working env
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure
pyproject.toml
Outdated
version = "0.1.0" | ||
description = "Add your description here" | ||
authors = [ | ||
{ name = "Manisha Bhandari", email = "[email protected]" } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Probably not needed, commit has all the necessary information
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think these informations are necessary as they serve as the main configuration of the project, but open for discussions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant the author details
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
project author should be moest-np.
I see a lot of effort being put into this, greatly appreciate it.
|
Rye manages the package, including the virtual environments and Python one needs, so I think it would be much easier to use Rye as a one-stop solution, as one won't need to worry about Python's virtual environments madness.
I agree that for the size of this project, it might be a bit cumbersome, but if we think from the dev's perspective, it might be better off with this one with the main rationale being separation and management of the packages, depending on the dev, test, and ... environment and managing the virtualenvs. After all, we should always opt for the betterment direction. Since we are already using the Streamlint app, I am sure there will be many more packages coming in the future. It's always good to have one less tech debt from the get-go. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work 🎉👏
pyproject.toml
Outdated
version = "0.1.0" | ||
description = "Add your description here" | ||
authors = [ | ||
{ name = "Manisha Bhandari", email = "[email protected]" } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
project author should be moest-np.
@horrormyth @sumanashrestha my install dependencies check has been failing since I no longer have requirements.txt file, can you update this code coverage check please? |
@manisha841, if you could add me to your fork, I could then push changes in a separate branch on your fork or maybe to this branch. |
2fe68ef
to
553c8e2
Compare
@manisha841 CI should be fixed now; it seems that the coverage is under 80% feel free to make changes |
Thank you, I will look into it further. |
Use the following commands to set up rye in your system.