-
-
Notifications
You must be signed in to change notification settings - Fork 363
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
Fixes: #3862; Added an example Python Support case study #3882
Conversation
Please clean this up to follow the Typescript example documentation. Your code and documentation is far too verbose, far too repetitive, and generally not sufficiently simplified to be suitable for educational purposes. I cannot fix every detail for you, you need to go and read through those examples yourself and fix your code and documentation to match |
Ok @lihaoyi sir, Thanks for reviewing My silly mistake made one test fail its just f string error 😔 No worries i will update the code and documentation |
@lihaoyi Sir Final Testing has been done...
Please pardon me if anything missed or done in wrong way... |
@lihaoyi Sir Let me tell you one thing that i have seen actually i installed Ubuntu in WSL and when i run it for the first time it just only provides python which is installed with only setuptools and nothing else (no pip , no venv and others ) So when i searched on internet i came to know that we have to install pip and venv using but now here comes the problem of permissions we cant get sudo for every task and even this is not the good practise so to tackle this problem i have used official guide for installing pip for users without requiring sudo permission and this will be installed for the user rather than the sudo i think this is a good approach, rest if you say i will follow your way... |
A Mill build process should not install anything outside of the project directory, unless it's some kind of cache like |
But Sir what about github actions tests ? i think they will fail (not sure) |
Sir Please see this image i just installed the ubuntu-24.04 version and when i tried these commands you can see that it don't comes with pip and venv Moreover, i am not downloading anything outside the T.dest actually i just downloaded |
You should be able to use |
ok then let me recall the updates again:
Thanks For Review Sir (This community is filled with helpful members❤️) |
@himanshumahajan138 yep! I expect this will take multiple rounds of back and forth review to polish into shape, but it's moving in the right direction so I think we'll eventually get there |
@lihaoyi sir, like i have used type checking in every example is that a good and right way Moreover i will update all of these reviews for every example |
Yes having typechecking in every example is right. The examples are meant to build upon each other, so we would expect them to get longer and more complex as it progresses down the list |
@lihaoyi Sir, finally I have Updated the Code as per reviews and now I think we can move further in review or merging part Hope you Like it... Thanks for Reviews Sir... |
|
||
*/ | ||
|
||
// Uncomment the Result variable line(Line Number:13) in the `src/main.py` code to see `typeCheck` error |
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.
Rather than using instructions here, we should use sed
to uncomment it in the Usage
block, and then run ./mill typeCheck
again to show the error that is generated
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.
actually sir i want to show error also but the problem is that if i write code for error then it will fail in ci test as this will me a major error from mypy side so how can i do this
please Guide me a little Bit Sir...
Took a review pass. Many of the comments apply to the following docs and examples as well |
@lihaoyi Sir, Ready for Next Reviews what i changed in last updates:
Thanks For Reviews Sir... |
@lihaoyi Sir is there something wrong with ci tests this unknown type of test case failed (not even related to my code changes) ? unknowingly, My PR Got ❌ Tag... |
@himanshumahajan138 I think this looks good. Just need to run |
Actually sir before my last commit ci don't run for .mill build files but now it do for these files also and its was my fault that i didn't checked it... Thanks sir for your guidance and reviews... |
AND HERE WE GO🎉 Everything Ready for Merge Lets make this happen... Thanks A Lot Sir ❤️ |
Pull Request
Added an example Python Support case study
Fixes: #3862
Description
Added
Python
Support for Mill From basic installation to multiple supports like virtual environment, mypy integration , pex integrationRelated Issues
Checklist
Additional Notes: (Code Copy and Plagiarism is Strictly Prohibited in case of Bounty)