ekinhbayar / gitamp Goto Github PK

https://github.com/ekinhbayar/gitamp/blob/master/src/Response/Results.php#L52-L54

Every event has an $action, $eventURL and a $message.

All these things end up being passed as strings to GithubEvent.

I think it would be nice to have a EventFactory which can build several events. So that each event in the above switch can have his own class. Including it's own parsing rules.

That way the entire switch and the filling of the events can be reduced to simply:

$events[] = $this->eventFactory->build($event);

• Do we really need auryn? Or is it easier to just build the deps ourselves as we do not have a deep stack?
• We need to allow users to have the server listen on several ips/ports (an exampleis both ipv4 and ipv6 support)
• Abstract the server away in its own class
• Maybe abstract the special repositories to a dedicated class instead of repeating it (maybe even make it a config?)
• Change new Type(1), etc to not make use of magic numbers
• Make exception extend the base project exception instead of the global \Exception
• Instead of asking for a concrete HttpClient class maybe ask for an interface instead?
• Everything that returns a \Generator right now should return a Promise
• 2f48b31#diff-1b3db3c3ba01baa1e657a2965f6771a6R30 should not be nullable, but instead be an empty results
• Instead ofusing the onclose on the client (2f48b31#diff-1b3db3c3ba01baa1e657a2965f6771a6R52) we can just log the disconnect after this loop 2f48b31#diff-1b3db3c3ba01baa1e657a2965f6771a6R77
  • No we do not, currently the websocket server does not handle disconnects correctly
• Test with lowest deps
• Test with highest deps
• Test with php 8 if travis made a working image yet
• Set up phpcs

While looking into #36 I noticed we are running the NativeDriver on production.

We should probably install @bwoebi's ext/uv.

The cause of this may or may not be #35

Feb 10 16:37:49 GitAmp-Ubuntu php[12165]: PHP Fatal error:  Allowed memory size of 134217728 bytes exhausted at /opt/php-src/ext/date/lib/timelib.c:42 (tried to allocate 320 bytes) in /opt/gitamp/vendor/amphp/aerys/lib/Ticker.php on line 64
Feb 10 16:37:49 GitAmp-Ubuntu php[12165]: PHP Fatal error:  Uncaught LogicException: Cannot run() recursively; event reactor already active in /opt/gitamp/vendor/amphp/amp/lib/NativeReactor.php:52
Feb 10 16:37:49 GitAmp-Ubuntu php[12165]: Stack trace:
Feb 10 16:37:49 GitAmp-Ubuntu php[12165]: #0 /opt/gitamp/vendor/amphp/amp/lib/functions.php(46): Amp\NativeReactor->run(Object(Closure))
Feb 10 16:37:49 GitAmp-Ubuntu php[12165]: #1 /opt/gitamp/vendor/amphp/aerys/lib/Process.php(129): Amp\run(Object(Closure))
Feb 10 16:37:49 GitAmp-Ubuntu php[12165]: #2 [internal function]: Aerys\Process->Aerys\{closure}()
Feb 10 16:37:49 GitAmp-Ubuntu php[12165]: #3 {main}
Feb 10 16:37:49 GitAmp-Ubuntu php[12165]:   thrown in /opt/gitamp/vendor/amphp/amp/lib/NativeReactor.php on line 52

I think the API would be nicer if this returns a promise.

The JS could use some love.

IMO several things should just be done on the serverside like (but not limited to):

• Capitalizing data
• The label texts

And some things can probably just be removed:

• Do we still want the dreamkiller functionality?
• Remove the global variables
• Remove unused playFromQueueExchange2

Some fixes / additions:

• Add some reconnect logic
• Add easter egg for events on this repo

Don't let the connection error exception bubble all the way but instead just return an empty Result object.

This may be hard to debug / find out when / why this happens so we probably want to introduce logging.

May I keep this issue open for issues and questions that come up when debugging/rewriting gitamp for amp v2 @kelunik, @trowski, everyone?

To start with, here is what I look at when I try to run it on v2_rewrite branch.

If I try to get the previous exception in BasicClient,
var_dump($dnsException->getPrevious()->getMessage());
It shows me string(27) "Multiple errors encountered", which is also on above gist. Btw, getReasons() is undefined, @kelunik. Am I doing something wrong here?

Currently the Results class here is tightly coupled.

We should introduce a ResultFactory add inject it into the contructor.

Need to check out the service config. Also maybe prepare a sample GAAS config.

Minify JS and CSS files

It would be nice if people are able to connect different event sources to the project.

This would involve making the interfaces more generic where needed and perhaps some other changes.

• Move the label texts to the backend
• This needs to implement a Listener interface
• Replace hardcoded namespace in the event factory

What is the nbsp doing here: https://github.com/ekinhbayar/gitamp/blob/master/public/index.html#L37?

• write all missing tests
• set up travis
• set up scrutinizer
• add commit bot?
• add CS checker on travis builds?
• add badges to the readme

Currently the redis counter expects passing in the key.

Is this really needed? I was thinking we could just create the following API instead:

interface Counter {
    public function increment(): Promise;
    public function decrement(): Promise;
    public function get(): Promise;
    public function set(int $val): Promise;
}

This way we don't have to pass in the key as a string.

When restarting the server the connected users are not being reset in the redis storage.

This results in inflated user counts.

Maybe a link on the GitAmp title?

Instead of using a temporary variable ($fn) just call resolve directly.

• expose address(es)
• expose port
• allow a way to add multiple origins

Re: http://chat.stackoverflow.com/transcript/message/34943048#34943048

Currently all "unknown" events are logged which makes it log way too much useless data.

We should change the log level to DEBUG here and change the minimum log level to INFO here.

@danschenker and I had this neat idea that I'll try to describe below.

Add a toggle that switches between the 'serene' vs 'rock' modes.

Current notes that are played for each event will be the 'serene mode' sounds and for the 'rock' mode we will need new sounds, perhaps some nice and short riffs? same notes but from a distorted electro guitar? Any cool sound that would do :-)

The toggle should also change the logo from the current one (for serene mode) to the neon logo

Add a PSR logger. This is especially needed for #30

Store the volume level in localstorage so it will be set correctly after revisit / reload.

Maybe we should just remove the header element and add the title / slider / etc just on top of the svg like the listener count.

That way the SVG bubbles aren't being cutoff by the header.

e.g. https://github.com/ekinhbayar/gitamp/blob/master/public/css/main.css#L204 vs https://github.com/ekinhbayar/gitamp/blob/master/public/css/main.css#L208

We need to introduce some tooling to force / check for fq function calls in the project.

We already have been looking at pre-commit hooks to check for and fix this using https://github.com/nilportugues/php-backslasher. But commit hooks are either only locally and cannot be added to the repo or involve git fuckery with custom hooks directories.

We also tried some way of CI integration, but without much success.

If somebody has some good ideas about this suggestions are very welcome.

The latest version of phpstorm support inspections for this so at the very least we should setup and add the IDEA config to the project which includes this inspection if possible.

Better we hide that one, looks like debugging is in place still =)

Do we need a base exception (ekinhbayar\GitAmp\Exception) from which all project exceptions inherit?

These can be made private / protected: https://github.com/ekinhbayar/gitamp/blob/master/src/Events/GithubEvent.php#L21-L29

Setup TLS on the server

Change the color of the addressbar on mobile to the background color of the page.

immersive++

Currently the log level is hard coded.

Let's be nice and move it to the config.

We should just let auryn compose the gitamp object here

There are some scanners doing post requests to random URLs.

This breaks our glorious project.

Example crash output:

GitAmp-Ubuntu php[12165]: [2017-02-10 10:06:37] critical LogicException: Cannot update resolved promise in /opt/gitamp/vendor/amphp/amp/lib/PublicPromisor.php:28
GitAmp-Ubuntu php[12165]: Stack trace:
GitAmp-Ubuntu php[12165]: #0 /opt/gitamp/vendor/amphp/aerys/lib/Server.php(591): Amp\Deferred->update('cmd=%63%64%20%2...')
GitAmp-Ubuntu php[12165]: #1 /opt/gitamp/vendor/amphp/aerys/lib/Server.php(567): Aerys\Server->onParsedEntityPart(Object(Aerys\Client), Array)
GitAmp-Ubuntu php[12165]: #2 /opt/gitamp/vendor/amphp/aerys/lib/Http1Driver.php(498): Aerys\Server->onParseEmit(Object(Aerys\Client), 4, Array, NULL)
GitAmp-Ubuntu php[12165]: #3 [internal function]: Aerys\Http1Driver->parser(Object(Aerys\Client))
GitAmp-Ubuntu php[12165]: #4 /opt/gitamp/vendor/amphp/aerys/lib/Server.php(555): Generator->send('POST /command.p...')
GitAmp-Ubuntu php[12165]: #5 /opt/gitamp/vendor/amphp/amp/lib/NativeReactor.php(265): Aerys\Server->onReadable('000000003401e4c...', Resource id #88544, Object(Aerys\Client))
GitAmp-Ubuntu php[12165]: #6 /opt/gitamp/vendor/amphp/amp/lib/NativeReactor.php(249): Amp\NativeReactor->doIoCallback('000000003401e4c...', Object(stdClass), Resource id #88544)
GitAmp-Ubuntu php[12165]: #7 /opt/gitamp/vendor/amphp/amp/lib/NativeReactor.php(214): Amp\NativeReactor->selectActionableStreams(0.8837)
GitAmp-Ubuntu php[12165]: #8 /opt/gitamp/vendor/amphp/amp/lib/NativeReactor.php(71): Amp\NativeReactor->doTick(false)
GitAmp-Ubuntu php[12165]: #9 /opt/gitamp/vendor/amphp/amp/lib/functions.php(46): Amp\NativeReactor->run(Object(Closure))
GitAmp-Ubuntu php[12165]: #10 /opt/gitamp/vendor/amphp/aerys/bin/aerys(99): Amp\run(Object(Closure))
GitAmp-Ubuntu php[12165]: #11 {main}

We should catch these and others and properly return a 404 response.

Imo it would make sense to also commit the composer.lock file so that every developer will have the same deps.

Just putting it here in case somebody ever hits something related:

notice Undefined offset: 0 in /opt/gitamp/vendor/amphp/socket/lib/functions.php on line 90

No idea what's going on. Maybe it was a glitch / connection hickup, but I doubt it should throw notices in our face.

It started working again after restart. We need to keep an eye on it to see if it goes funky again.

Move these to a dedicated class (e.g. GithubEventType) which includes a method isValid().

This method should be used here.