mavo Consistent function naming: let's go with underscores

Consistent function naming: let's go with underscores

Open LeaVerou opened this issue 2 years ago • 10 comments

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.

Oct 28 '21 11:10 LeaVerou

Is this up-for-grabs? Would love to contribute on this :)

Oct 28 '21 18:10 amm98d

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

Oct 28 '21 18:10 karger

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.

Oct 29 '21 15:10 LeaVerou

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.

Oct 29 '21 16:10 LeaVerou

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.

Alright, will send PR soon !

Oct 29 '21 19:10 amm98d

Hi I would like to contribute, are you still open for help?

Nov 02 '21 13:11 jvitor211

Hi I would like to contribute, are you still open for help?

Sure! How did you find this project, if I may ask?

Nov 02 '21 13:11 LeaVerou

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 .

Nov 02 '21 14:11 jvitor211

I would like to contribute on this. This is going to be my first contribution, therefore can you please tell me where to start.

Dec 12 '21 14:12 NEERAJthinker

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() and tofirst() 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. 🙇 👍

Jan 23 '22 21:01 norbitrial

@LeaVerou I would like to contribute on this. Assign me.

Feb 07 '23 10:02 FirdausJaimi

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.

Feb 14 '23 19:02 DmitrySharabin

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 🚀

Mar 07 '23 19:03 norbitrial