Contributors

[snapshotpl]

Site about us!

[Ocramius]

The convention is underscores for keys of configs

[localheinz]

@snapshotpl

Agree with @Ocramius' suggestion!

Also, as far as I can see, this configuration is used by

• Controller\ContributorsController
• View\Helper\GitHubUrlFactory

and I'm not sure whether that's a good thing!

See, if you changed the key zf-modules to anything, it would require a change now in these two factories, maybe this is asking for a service which returns a value object? Sounds like overkill, but maybe it's better?

return [
    'zfmodules_repository' => [
        'owner' => 'zendframework',
        'name'  => 'modules.zendframework.com',
];

<?php

namespace ZfModule\Entity;

class Repository
{
    private $owner;
    private $name;

    public function __construct($owner, $name)
    {
        $this->owner = $owner;
        $this->name = $name;
    }

    public function owner()
    {
        return $this->owner;
    }

    public function name()
    {
        return $this->name;
    }
}

And then the factories just retrieve that thing and use it for whatever they need to do with it?

[Ocramius]

zf_modules is also a bit generic IMO

[Ocramius]

Looks sane, tho tests are missing

[localheinz]

@snapshotpl

Should be Contributors, you're missing an r here. 😉

[localheinz]

@snapshotpl

The page looks a bit empty at the moment

Do you think it would make sense - other than repeating GitHub's contributors page - to show a randomized list of contributor avatars?

[snapshotpl]

@localheinz there should be pretty nice list of 36 avatars. I don't know what happend

[localheinz]

@snapshotpl

Dang, snapshotpl#8 left the PR in an unmergeable state, probably because of the changes in #445.

[localheinz]

@Ocramius

Since this includes contributions from me, do you want to take a look again?

@snapshotpl

If @Ocramius has no objections, I would be happy to merge and deploy, we can still re-iterate!

[Ocramius]

@localheinz it needs a rebase (with --preserve-merges, careful!)

[localheinz]

@Ocramius

About the rebase, shall I do it before merge or ask @snapshotpl to do it?

[Ocramius]

Your decision :-)
On Mar 4, 2015 8:33 AM, "Andreas Möller" notifications@github.com wrote:

@Ocramius https://github.com/Ocramius

About the rebase, shall I do it before merge or ask @snapshotpl
https://github.com/snapshotpl to do it?

—
Reply to this email directly or view it on GitHub
#447 (comment)
.

[localheinz]

@snapshotpl

Rebasing and merging this in a minute!

[localheinz]

Merged with 5a77073.

[localheinz]

And, thank you very much, @snapshotpl!