RFC: Disallow combining use and const statements (accepted)

4 years ago

Jan Tvrdík
Nette guru | 2550
+
+51
-

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

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

4 years ago

Felix
Nette Core | 907
+
0
-

I like proposed state. (+1 for RFC)

4 years ago

Filip Procházka
Moderator | 4693
+
0
-

Yes please!

4 years ago

Šaman
Member | 2294
+
0
-

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

4 years ago

Milanov
Member | 50
+
0
-

Yes, +1.

4 years ago

Northys
Member | 28
+
0
-

yes! +1

4 years ago

kukulich
Member | 40
+
0
-

Yes! +1

4 years ago

Patrik Votoček
Member | 2249
+
0
-

Yes! +1

4 years ago

wodCZ
Member | 49
+
0
-

Yes! +1

4 years ago

MartinitCZ
Member | 590
+
0
-

Yes! +1

4 years ago

xificurk
Member | 119
+
+1
-

+

4 years ago

nAS
Member | 279
+
0
-

Yes! +1

4 years ago

Tomáš Votruba
Moderator | 1154
+
0
-

+1

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

4 years ago

Šaman
Member | 2294
+
+9
-

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

4 years ago

Milo
Nette Core | 1146
+
0
-

@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.

4 years ago

Milo
Nette Core | 1146
+
+1
-

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 :)

4 years ago

Tomáš Votruba
Moderator | 1154
+
+2
-

@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.

4 years ago

Jan Tvrdík
Nette guru | 2550
+
+2
-

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;

4 years ago

David Grudl
Nette Core | 6846
+
0
-

To not duplicate docblocks is against phpDoc standard.

4 years ago

Jan Tvrdík
Nette guru | 2550
+
+2
-

@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)

4 years ago

David Grudl
Nette Core | 6846
+
+2
-

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?

4 years ago

hrach
Member | 1812
+
0
-

@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.

4 years ago

David Grudl
Nette Core | 6846
+
0
-

@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…

4 years ago

hrach
Member | 1812
+
+2
-

@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.

4 years ago

Jan Tvrdík
Nette guru | 2550
+
+8
-

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.

4 years ago

castamir
Member | 631
+
+9
-

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.

4 years ago

David Grudl
Nette Core | 6846
+
+3
-

@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.

4 years ago

Jan Tvrdík
Nette guru | 2550
+
0
-

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

4 years ago

David Grudl
Nette Core | 6846
+
0
-

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

4 years ago

Tomáš Votruba
Moderator | 1154
+
+2
-

@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)

4 years ago

Milo
Nette Core | 1146
+
-1
-

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