Usability: Improve "tools/mpy_cross_all" to invoke the "mpy-cross" program coming from the mpy_cross distribution #4956
Conversation
if installed within the current environment instead of invoking directly
|
Interesting, but despite the commit message title the commit does two unrelated things making it pretty hard to review: you're both introducing new functionality and refactoring the file to use functions etc. The latter should be a separate commit. For clarity, but also because suppose it isn't wanted it can just be dropped without having to change the other commit. Also the commit message doesn't adhere to project standards (missing dot at the end of the first line, body should be complete sentences I guess). |
|
Dear @stinos, thanks for your answer. Despite having tried hard already to be compliant with the MicroPython contribution guidelines, we will be happy to amend the patch according to your suggestions. However, we would like to mention that the outlining of this process has a very nitpicky tone in it and would get a totally different and very much positive attitude when just telling newcomers that this are actually nits when compared to the respective content of the contribution. Otherwise, as you will see when reviewing your own comment, the immediate talking about process related things occupying 95% of your post will definitively and instantly blast away much motivation for many newcomers. The motivational aspects of coming here and finding a way climbing all those steps onto the hills of MicroPython development already should receive more attention from the core developers, which I am counting you in. We are usually applying high standards to our software development process and commit style so we are a bit surprised about the overinstructive tone. Saying that, we don't want to downplay the importance of nits in code reviewing but also would like to mention what they just are: nits ;]. Nevertheless, we are very happy that you are taking the time to look at our contribution and will definitively continue trying to make the contribution adhere to the project standards. With kind regards, |
|
@amotl apologies if my comment sounded overly negative, it was not meant as such, just wanted to explain how this PR can be improved so that when it gets an actual review by dpgeorge he can spend time on looking at the actual implementation without having to worry too much about less important aspects, i.e. the things I mentioned. Though in that light, I do think the issue about splitting the commit is just a bit more than just a nit as it really makes the review process harder, and I hope you understand that. But indeed I could have mentioned explicitly these are details in comparision with the actual functionality changed. |
|
For reference, the distribution of mpy-cross on pypi is not in any way official (as far as the Micropython Project or dpgeorge is concerned). As such it's possibly undesirable to have the official tools referencing the un-official distribution at all. More importantly though, if you have the micropython source already you should always be able to compile mpy-cross for yourself quite easily, then you'll be guaranteed the version of it correctly matches the kernel being compiled. |
|
Still, it wouldn't be bad to allow people to run whatever mpy-cross method they want. I think I've seen questions like 'please provide mpy-cross binaries' more than once, so I guess for beginners saying 'download source, pip install, run' is easier than 'make'. But thinking about it for a bit, I now feel the approach in this PR is not suitable. Instead there is a simpler yet more generic solution: be to leave the original script intact, but with one change only: instead of hardcoding |
|
Dear @andrewleech and @stinos, thanks for your valuable insights, for listening to what's behind our suggestion and for extrapolating it to similar demands you already recognized from the community through respective questions you have seen already. For what I am concerned about, @stinos last comment absolutely makes sense to us and we will adjust this PR accordingly. While there would still be room for improvements to automate things like we will be happy to put this last mile into an external package similar to what @dhylands did with wrapping the REPL through We recognize you are busy keeping up the spirit and the very details of MicroPython so we don't want to bother you too much with our rambl 8000 ings. However, if you also like where we are aiming at here and outlined a bit through #4955 in general, we will be very happy if you could share some of your thoughts from time to time by regularly stepping by when your time permits. In fact, we strive for making certain things easier for absolute beginners but well would like to improve other things for advanced users as well. Thanks again and with kind regards, |
|
I'm also quite open to adding such functionality into my pypi package, or I'd suspect that mpy-cross could probably have python glob and/or walk support added directly to it at allow multiple file/folder parsing. |
|
Thanks for this, but I think it's too complex. Alternatives to get the same functionality:
|
|
I've added a ticket for what I think would be a good alternative to this feature in my package tracker, not sure if/when I will have a chance to look at implementing though https://gitlab.com/alelec/mpy_cross/-/issues/13 |
|
Thank you very much!
…On 7 July 2021 13:40:47 CEST, Andrew Leech ***@***.***> wrote:
I've added a ticket for what I think would be a good alternative to
this feature in my package tracker, not sure if/when I will have a
chance to look at implementing though
https://gitlab.com/alelec/mpy_cross/-/issues/13
--
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#4956 (comment)
--
Sent from my mind. This might have been typed on a mobile device, so please excuse my brevity.
|
Dear MicroPython developers and community,
Introduction
Coming from #4955 and building upon #3040 and #3057, we enabled
tools/mpy_cross_all.pyto invoke thempy-crossprogram through the finempy-crossdistribution package.Setup
The operator will be able to decide to selectively install specific versions of
mpy-crosslikesource .venv2/bin/activate pip install mpy-cross=1.9.4into the Python environment running on her workstation.
Usage
Description
When running it from a Python environment where the
mpy_crossmodules is installed,mpy_cross_all.pywill attempt to invokempy-crossfrom thempy_crossdistribution.Otherwise, it will go down the established route of invoking
mpy-crossdirectly, essentially expecting it to reside somewhere on$PATH.Synopsis
We hope you like this.
With kind regards,
Andreas.