trident icon indicating copy to clipboard operation
trident copied to clipboard

rename all functions with _ to proper names

Open morrowc opened this issue 8 years ago • 5 comments

https://talks.golang.org/2014/names.slide#6

oops...

trident$ grep 'func ._.' src/lib/.go | wc -l 32 trident$ grep 'func ..' src/ui/.go | wc -l 17 pitchfork$ grep 'func .*.' lib/.go | wc -l 342 pitchfork$ grep 'func ._.' ui/*.go | wc -l 149

and of course those will all have to be chased up and fixed in all other callers :( I'm happy to take a stab at this, once I sort out how to actually test/compile the lot of this.

morrowc avatar Feb 14 '17 05:02 morrowc

That's a pandoras's box that we'll get to, but we have other things to focus on, I'd make this a low priority.

bapril avatar Feb 14 '17 13:02 bapril

At Tue, 14 Feb 2017 05:29:33 -0800, bapril [email protected] wrote:

[1 <text/plain; UTF-8 (7bit)>] That's a pandoras's box that we'll get to, but we have other things to focus on, I'd make this a low priority.

agreed it's a pandora's box... that's why I said I'd be happy to try and tackle it outside of whatever things you all are up to.

-- You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub: https://github.com/tridentli/trident/issues/80#issuecomment-279706992 [2 <text/html; UTF-8 (7bit)>]

morrowc avatar Feb 14 '17 14:02 morrowc

One other big reason not to is because most of those functions are not really go-style functions as in the common Go sense.

Many are CLI commands others (H_/h_) are UI commands, they are not to be referenced outside the package anyway.

We'll likely get to a good part of changing this though with https://github.com/tridentli/pitchfork/milestone/3 when we change everything into separate packages, the way that is more go-style.

massar avatar Feb 16 '17 11:02 massar

I don't agree that 'not to be referenced outside the package' is a valid reason to avoid code quality/readability... If I send along pull requests for renames, will they be accepted? So far the renames I've got in my local repository are pretty simple, just there are a bunch of them :)

morrowc avatar Feb 21 '17 15:02 morrowc

I am all pro code quality and readability. The format of the CLI and UI functions grew over time (before we checked that go likes it differently).

We will re-architect things when 2.0 comes around, see amongst https://github.com/tridentli/pitchfork/milestone/3

and when that comes we'll modularize/package things and we'll also take care of all the non-standard golang function names.

Thus, no, in the current version we will not be changing those. There are more important things to do.

massar avatar Feb 21 '17 16:02 massar