async-interop / event-loop

An event loop interface for interoperability in PHP.
MIT License
170 stars 9 forks source link

Line Length and Code Formatting #76

Closed AndrewCarterUK closed 8 years ago

AndrewCarterUK commented 8 years ago

We've pretty much cleared the issue and pull request queue now.

One of the tasks we were saving to this point is the line length on the phpdoc and when to wrap. I'm happy to go with any limit as long as we are consistent and it's not ridiculous (I can read it on GitHub without scrolling or wrap).

bwoebi commented 8 years ago

I can display ~140 chars on github … so, what about a (soft) limit of 120 chars and hard limit of 130 chars?

AndrewCarterUK commented 8 years ago

Sounds reasonable. Question for everyone - where does this snippet cut off on your screen:

https://github.com/async-interop/event-loop/blob/master/src/Loop/Driver.php#L29-38

    /**
     * Defer the execution of a callback.
     *
     * The deferred callable MUST be executed in the next tick of the event loop and before any other type of watcher. Order of definition MUST be preserved when executing the callbacks.
     *
     * @param callable(string $watcherId, mixed $data) $callback The callback to defer. The $watcherId will be invalidated before the callback call.
     * @param mixed $data Arbitrary data given to the callback function as the $data parameter.
     *
     * @return string An unique identifier that can be used to cancel, enable or disable the watcher.
     */
    public function defer(callable $callback, $data = null);
bwoebi commented 8 years ago

Uhm, github issues view is smaller, so for reference here: https://github.com/async-interop/event-loop/blob/master/src/Loop/Driver.php#L33

I see until "before the callbsnip"

AndrewCarterUK commented 8 years ago

On the comment it cuts off after "... the event loop and before...cuts" On the source file it cuts off after "... watcher. Order of defcuts"

bwoebi commented 8 years ago

That particular line is "Order of definition cuts" for me (in source)

kelunik commented 8 years ago

image

image

assertchris commented 8 years ago

Don't know if it's been mentioned elsewhere, but http://www.php-fig.org/psr/psr-2/#1-overview

bwoebi commented 8 years ago

soft limit of 120 chars is nothing else than what I've suggested … but that's really just coincidence as I don't really care about PSR-2…

assertchris commented 8 years ago

I'm just mentioning PSR-2 as it addresses this question. I don't think we've agreed (as a group) to ignore PSR-2, so I think it's relevant. Please be more courteous. :)

bwoebi commented 8 years ago

Sorry, too annoyed by the braces on following line … nothing personal, only sometimes angered about this PSR-2 thing ^_^

assertchris commented 8 years ago

I hate the inconsistencies too... :/

kelunik commented 8 years ago

I don't think we've agreed (as a group) to ignore PSR-2, so I think it's relevant. Please be more courteous. :)

Until now, we also didn't agree to follow PSR-2. ;-)

assertchris commented 8 years ago

@kelunik and I'm not suggesting we do. But it's still relevant to the discussion...

kelunik commented 8 years ago

I opened an issue to discuss this separately.

kelunik commented 8 years ago

I don't really understand PSR-2 here. Isn't SHOULD already the soft limit? Why is there a second soft limit as 120?

sagebind commented 8 years ago

Sorry, too annoyed by the braces on following line … nothing personal, only sometimes angered about this PSR-2 thing ^_^

You seem more overworked about it than necessary. It's only code formatting. I care more about what the code is than what it looks like.

120 seems like a reasonable wrap limit to me.

WyriHaximus commented 8 years ago

120 is very reasonable limit, even before that the longer a line gets the harder it becomes to read (rule of thumb, there are exceptions)

bwoebi commented 8 years ago

I've looked right now, there are no overlong lines left; closing.

kelunik commented 8 years ago

We still need to adjust the shorter ones to be consistent.