Following up a discussion on the maildev list about using 'aArg' style function parameters in Calendar code.

So I chatted with Paul on this briefly in IRC. I'm a fan of aArg since it allows to distinguish args from local variables and keeps people from reassigning parameters. I see there is general movement to get rid of it though, so I'd be open to it.

What we could do is enable eslint no-param-reassign as well, which could be some work to convert but would alleviate my concern about reassignment. MakeMyDay, what do you think?

I don't have a strong opinion either way. There is some benefit in distinguishing args from local variables. (Ideally that would be handled by tooling, say by editors giving them different colors. Not sure whether that exists.) If functions are kept short (usually a good idea) then there's less benefit to making the distinction. On the other hand, I think general readability is a little better without the aArgs, and not using them is more consistent with other TB and FF code.

So I have a slight preference for doing no-aArgs, if we also enable no-param-reassign and no-shadow (which would be a good idea in any case, as pcloke noted in the maildev thread).

As you'd guess from the the maildev thread, I'm of course in favour of dropping aArg style. At least phasing it out for new code.

Not sure I understand the the concern about distinguishing local variables from parameters. For the absolute majority of the cases I'd think that is basically a non-issue since the parameters are primitives (i.e. reassigning isn't much of an issue). Maybe someone can enlighten me? Yes I read the article linked from the no-param-reassign eslint page - seems like a very special case to me.

Agreed we can enable no-param-reassign and we should also opt for { "props": true } so there's no out-param usage in our functions. While that is possible it's clearly to be considered a bug.

I think we should not enable the no-aArgs rule and also not migrate away from it. I have not a preference whether to make the argument notation consistent for calendar, but if we want to do this, I have a strong preference to go with aArg notation and not without it.

aArg notation is a good instrument to see at a glance whether a variable is/should be a passed argument that didn't change within a function. The tpye of variable doesn't matter here. That adds code readibility.

I see we violate the mentioned principle at some places in calendar, but that is not an argument against the principle, but to fix those reassignments. That said, I'm in favour to enable the no-param-reassign rule (with or without the out-param check), too.

As a general note and already mentioned elsewhere, I think not everything that rolls down the hill should be picked up. If other modules or m-c has different preferences for linting they are free to implement it, but not put it onto other modules like we're not trying to force our set of linting rules to others. To achieve code style consistency with other modules in all dimensions possible should explicitely not be a goal for calendar from my point of view.

(In reply to [:MakeMyDay] from comment #4)

To achieve code style consistency with other modules in all
dimensions possible should explicitely not be a goal for calendar from my point of view.

Can you elaborate on that? "Global" consistency is the ultimate goal of linting so that it is as easy as it can be for someone new to just pick up and fix something. Having to switch styles is a rather large barrier.

Sure. Consistency is of cause the goal of linting, but the question is to what extend - so it's a matter of defining "global" here. Not every rule is universally used or reasonable on each and every project you find on the internet, not to mention all of the commercial software development projects. This is typically and rightly something that is defined inidividually to meet the needs and habits.

You have always to get accustomed to the code base you're starting to work on. So, having the ruleset defined on module level fits perfectly, so to that regard the module is my definition of "global" in general and calendar in this case. And calendar doesn't use an alien ruleset, so that is not too hard to follow.

We have much larger barriers for new contributors then this like getting to a build at all for the first time, finding up-to-date developer information about the process until you get your first patch checked in or to get even a rough understanding how things (especially the Mozilla specific technologies) are working. Before people could even start being annoyed by a linting rule, most of them have given up already.

And once you got beyond that point, you're used to run mach lint -l eslint <module> on your patch anyway to at least catch typos. As a reviewer, I expect this has happend before asking for review. So, you already should get told if you didn't follow the rule for the module you're working on.

What you describe assumes people work on one module and one module only. Many cases that may end up being true, but I think a non-goal. For staff making changes across the product, differences are certainly confusing.

Judging from bugzilla, staff working on calendar these days should be used to calendar linting rules as long time contributors or former calendar GSoC students.

Most will work across the Thunderbird product, including lightning.
Knowing some rule exists is different from having to adjust per patch. (In mail/ we use the standard mozilla rules - and yes I don't necessarily like all of them.)