mavo
mavo copied to clipboard
Consistent function naming: let's go with underscores
Currently we have very few MS functions with 2 word names (thanks @DmitrySharabin for hunting them down):
-
groupby()
-
tolast()
-
fromlast()
Given that Mavo is generally case insensitive, I don't think concatenating names makes for very readable code. Instead, we should use underscores to separate words, which is guaranteed to be readable with any casing.
A solid plan for this would be:
- Any new multi-word functions use underscores (e.g.
readable_datetime()
) - We rename the three functions above to
group_by()
,to_last()
,from_last()
We add the current names as aliases, which call Mavo.warn()
to print out something like:
groupby() is deprecated and will be removed in the next version of Mavo. Please use group_by() instead.
and then call the new function name.
We need to also make sure we're not calling the old names in any of our own code or plugins.
Is this up-for-grabs? Would love to contribute on this :)
I took a look at the google sheet functions; they are interestingly inconsistent between no separator, underscore, and dot (perhaps they're doing some namespacing of functions). May be worth a look for comparison: https://support.google.com/docs/table/25273?hl=en . Of course, most gsheet functions are copying excel functions, so the inconsistency is probably Microsoft's fault: https://support.microsoft.com/en-us/office/excel-functions-alphabetical-b3944572-255d-4efb-bb96-c6d90033e188
Is this up-for-grabs? Would love to contribute on this :)
Sure, thanks! One caveat is that while most of these are defined in src/functions.js
, the groupby()
function is defined in src/mavoscript.js
because it's also an operator.
I took a look at the google sheet functions; they are interestingly inconsistent between no separator, underscore, and dot (perhaps they're doing some namespacing of functions). May be worth a look for comparison: support.google.com/docs/table/25273?hl=en . Of course, most gsheet functions are copying excel functions, so the inconsistency is probably Microsoft's fault: support.microsoft.com/en-us/office/excel-functions-alphabetical-b3944572-255d-4efb-bb96-c6d90033e188
Yup, I've already looked there and decided that since there's no consistent precedent, I'll go with whatever seems most legible.
Is this up-for-grabs? Would love to contribute on this :)
Sure, thanks! One caveat is that while most of these are defined in
src/functions.js
, thegroupby()
function is defined insrc/mavoscript.js
because it's also an operator.
Alright, will send PR soon !
Hi I would like to contribute, are you still open for help?
Hi I would like to contribute, are you still open for help?
Sure! How did you find this project, if I may ask?
a few days ago I was on the csail MIT website and found this research group and commented with the professor, Mr. David Karger. And I talked about the project asking if I could be part of it in any way and contribute
https://prnt.sc/1y67p4j https://prnt.sc/1y67gjx
Em ter., 2 de nov. de 2021 Γ s 09:58, Lea Verou @.***> escreveu:
Hi I would like to contribute, are you still open for help?
Sure! How did you find this project, if I may ask?
β You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/mavoweb/mavo/issues/793#issuecomment-957635065, or unsubscribe https://github.com/notifications/unsubscribe-auth/AJ622NDOPNFSTBAYEVFY7FTUJ74CPANCNFSM5G4T6MPA .
I would like to contribute on this. This is going to be my first contribution, therefore can you please tell me where to start.
Currently we have very few MS functions with 2 word names (thanks @DmitrySharabin for hunting them down):
groupby()
tolast()
fromlast()
Given that Mavo is generally case insensitive, I don't think concatenating names makes for very readable code. Instead, we should use underscores to separate words, which is guaranteed to be readable with any casing.
π Hey @LeaVerou! Not sure if help is still needed here but I've opened a PR as above. π
It tenned to add deprecation warnings for the old function calls meanwhile introducing the underscore version of them. Function calls are tested on my local, everything seems to be working exact the same way as before. There are extra warnings on the console for old function calls for the following cases:
-
groupby()
function call, -
fromlast()
andtofirst()
calls.
I assume these are the functions which are planned to renamed. I couldn't really find tolast()
.
Let me know if you have any suggestions about the PR or anything else need to be added. π π
@LeaVerou I would like to contribute on this. Assign me.
I couldn't really find
tolast()
.
Hey @norbitrial,
You are absolutely rightβthere is no such function. The correct name is tofirst()
, and you've already fixed it. Thank you!
If you are still interested in contributing (which would be awesome), and @LeaVerou doesn't mind, we can work through your PR together.
If you are still interested in contributing (which would be awesome), and @LeaVerou doesn't mind, we can work through your PR together.
I am still happy to help here. Let me know what's required from my side there in the PR which could be found here: https://github.com/mavoweb/mavo/pull/821 π
I am still happy to help here. Let me know what's required from my side there in the PR which could be found here: #821 π
That's great! Thanks. π
can i contribute to this project
@LeaVerou Can I contribute to this please.. I would love to do it :).
Hey would love to contribute, please assign this issue to me
Hey! I am beginner. Would love to contribute my first contribution in open source. Please assign this issue to me.
hey I'm unable to find groupby function in this repo.
Hey, how can I contribute to this?