RFC: Disallow combining use and const statements (accepted)

Jan Tvrdík

Disallow combining use and const statements

This RFC proposes to update the current coding standards to disallow combining use and const statements

Current state

use Nette,
	Nette\Application,
	Nette\Utils\Strings;

const HOST = 1,
	PATH = 2,
	RELATIVE = 3;

Proposed state

use Nette;
use Nette\Application;
use Nette\Utils\Strings;

const HOST = 1;
const PATH = 2;
const RELATIVE = 3;

Rationale

Cleaner diffs.
Consistent alignment.
It works with PhpStorm's automatic imports.
Supported by Jakub Vrána

Last edited by Jan Tvrdík (2014-11-08 17:51)

Felix

I like proposed state. (+1 for RFC)

Filip Procházka

Yes please!

Šaman

Hmm, i use first variant, but there are no reason for it and there are some minor reason for second variant. So +0.5 :)

Milanov

Yes, +1.

Northys

yes! +1

kukulich

Yes! +1

Patrik Votoček

Yes! +1

wodCZ

Yes! +1

MartinitCZ

Yes! +1

xificurk

+

nAS

Yes! +1

Tomáš Votruba

+1

(very nice text – simple, clear, 1 change at a time)

Šaman

BTW, why we each create new +1 post, when we can simply vote by like-counter? :)

Milo

@JanTvrdík Why do you propose the change? Sure, I read your rationales but do you have some problem with current state personally?

This change does not affect framework users. They simply require autoloader.php and who cares. And there is the API documentation.

This change affects only the framework developers. Did you hit some issue why the current state breaks your dev workflow?

Personaly, I'm changing this coding style from project to project. Both states are OK for me.

And one note for separated const. You must duplicate docblock too when it is the same for all constants.

Milo

Guys with -1 vote :) I'm respecting your votes, but wouldn't be better to say some arguments? Moreover when half of my post are questions :)

Tomáš Votruba

@milo Wrote:

This change affects only the framework developers. Did you hit some issue why the current state breaks your dev workflow?

I'd remove “only” from the first sentence. Developers are the most important people in package evolution.

I agree with mentioned pros (positively marked issues):

@JanTvrdík wrote:

Cleaner diffs.
Consistent alignment.
It works with PhpStorm's automatic imports.

and add every unnecessary complexity makes code smell, thus more difficult to work with.
Here it's 3 cases instead of 1.

Jan Tvrdík

do you have some problem with current state personally?

A little. The @param RFC is however much more important and if we are going to update the entire Nette code base because of it, we may improve this as well.

You must duplicate docblock too when it is the same for all constants.

Not really. It has very little benefit for static analysis, making it understandable for users should be enough, e.g. this should be fine:

/** @internal url type */
const HOST = 1;
const PATH = 2;
const RELATIVE = 3;

David Grudl

To not duplicate docblocks is against phpDoc standard.

Jan Tvrdík

@DavidGrudl As I've said in the first post – phpDoc should serve primary the user, not the phpDoc standard. In some cases I believe it may by truly better (for benefit of the user) to have one phpDoc block for each constant, in the other cases we may use phpDoc templates (revert 2169d24)

Last edited by Jan Tvrdík (2014-11-05 08:04)

David Grudl

I don't understand. This

/** @internal url type */
const HOST = 1;
const PATH = 2;

is completelly different from this

/** @internal url type */
const HOST = 1;
/** @internal url type */
const PATH = 2;

For me, as user, the generated API and hinting in IDE will differs for const PATH. So when phpDoc is primary for user, the annotation must be doubled (or written as template) always. In which cases it is not needed?

hrach

@DavidGrudl yes, it is! But just in machine-view way! As a user, it' pretty clear, that “near” definitions share the comment. However, I'm ok with repeating the comment.

David Grudl

@hrach So your definition of “user” is somebody who looks into the source code and doesn't use IDE or api.nette.org. This is not very common…

hrach

@DavidGrudl yes, thats my workflow. I don't use api.nette.org at all. Personally I'm going through the code, with IDE or with github (key t), watching the real code.

Jan Tvrdík

In which cases it is not needed?

OK, I changed by mind. We should always use either phpDoc template (if one information for all constants is enough) or write extra phpDoc for each constant.

castamir

This might looks like an off-topic, but to me, these RFCs are meaningless in the current form, because it looks like there is only one person to decide, whenever they will be accepted, denied or totally ignored for eternity (no offense @DavidGrudl =)).

I realy miss some kind of community desicion making, some kind of “elders” voting or whatever. For example this RFC has so many supporting votes from community but what now? Will it be approved / denied? Who else will make the decision (except @DavidGrudl )? When? It would be nice to have some kind of “official” feedback from Nette Foundation Member(s) (btw I haven't seen any report from NF meetup since April).

It would be also nice to have a roadmap of approved features to be implemented. And perhaps a slightly different attitude to community members to motivate them to implement these features or help motivated but less skilled ones to do so.

David Grudl

@castamir We can vote on it. This RFC will affect only those who develop Nette, not those who use it, so vote weight should be based on the count of commits to Nette, for example in the last year.

Jan Tvrdík

@DavidGrudl So, what do you think about this? It is now or never.

David Grudl

It would be useful to do it in Nette 2.1 (and perhaps 2.0), to facilitate cherrypicks and comparison of code.

Tomáš Votruba

@castamir You hammered the nail, thanks. I really agree that Nette needs to change this to support more finished RFC and activity from the community.

I'm also curious about evolution of NF meetups.

David Grudl wrote:

@castamir We can vote on it. This RFC will affect only those who develop Nette, not those who use it, so vote weight should be based on the count of commits to Nette, for example in the last year.

That would be great and also motivating for contributors.

Last edited by Tomáš Votruba (2014-11-06 14:49)

Milo

Topic about RFC voting. Sorry, in Czech language only.