-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
70%: Introduce namespaces #56
70%: Introduce namespaces #56
Conversation
Should MongoGraph and Extended graph simply be |
It bugs be that Apart from the obvious |
I do like that. Part of me kind of thinks that |
ITripodSearchProvider -> ISearchProvider +1. We are using capital I right? |
jobs should have their own package. |
Yes, |
Composites should have their own package, i.e. |
@RobotRobot I can easily buy |
|
i.e. |
(also, in what dialect is 'Tripod' a verb? Is this a Staffordshire thing?) |
It's not a verb. But it's not a common noun in the sense that it describes the purpose of the class, if you see what I mean. I like Client. |
Oh, I see, right. I agree that the 'library' is Tripod, not the class. |
Another idea (not necessarily better than client, and keep them coming) we could keep ITripod, but |
I'm not sure I like |
re. Composites, just general tidyness. That package looks crowded. I would drop the |
Actually, the more I think of it, the more I like the argument behind 'mongo Driver for iTripod. |
Right that's it in terms of feedback from me today, will be offline rest of day so if you need to get this merged you have my opinions... ;-) |
Ok, that's reasonable. When spiking this out, I was concerned that it was just pushing the clutter to the namespaces (and that that might be harder for the uninitiated to navigate), but I don't have a strong opinion on that (especially now that I'm on the other end of the process) |
@@ -82,10 +82,10 @@ function showUsage() | |||
*/ | |||
function generateTables($id, $tableId, $storeName, $stat = null) | |||
{ | |||
$tableSpec = MongoTripodConfig::getInstance()->getTableSpecification($storeName, $tableId); | |||
$tableSpec = TripodConfig::getInstance()->getTableSpecification($storeName, $tableId); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We seem to have \Tripod\Mongo\Config in other places, but TripodConfig here. Is this correctly named?
@@ -1,4 +1,11 @@ | |||
<?php | |||
|
|||
namespace Tripod; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We've prefixed the namespace in another lib with Talis
- do we want to do something similar here?
https://github.com/talis/persona-php-client/blob/master/src/Talis/Persona/Client/Login.php#L2
If we do it'll need an update to the composer.json file in order to work
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not terribly keen on this, but I'll defer to @RobotRobot
@@ -251,7 +253,7 @@ public function testGenerateViewWithCountAggregate() | |||
) | |||
); | |||
|
|||
$actualView = MongoTripodConfig::getInstance()->getCollectionForView('tripod_php_testing', 'v_counts')->findOne(array('_id'=>array("r"=>'http://talisaspire.com/works/4d101f63c10a6',"c"=>"http://talisaspire.com/","type"=>'v_counts'))); | |||
$actualView = \Tripod\Mongo\Config::getInstance()->getCollectionForView('tripod_php_testing', 'v_counts')->findOne(array('_id'=>array("r"=>'http://talisaspire.com/works/4d101f63c10a6',"c"=>"http://talisaspire.com/","type"=>'v_counts'))); | |||
// var_dump($actualView); die; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove the var_dumps
… Jobs and Composites
Conflicts: scripts/mongo/createTables.php src/mongo/Config.class.php src/mongo/delegates/TransactionLog.class.php src/mongo/delegates/Views.class.php test/unit/mongo/MongoTripodTablesTest.php
From the feedback, I have:
Any other comments before I merge into the feature branch? |
We're renaming the interface to IDriver too? |
Should we not? It's the interface for a Tripod Driver, right? |
I guess. I'm not going to argue about this too much given the chainable API #23 should be the right way to use tripod in the future. |
Agreed. I am certainly not married to the change: I just thought it seemed more consistent. I could be just as happy (well, ambivalent, really) with either name. |
This branch introduces namespaces to Tripod.
Feel free to argue about which classes should go in which namespaces and how they should be named.
The way I've divided them up currently (although this is totally open for discussion) is:
Tripod
ITripod
IDriver
ITripodStat
ChangeSet
ExtendedGraph
Labeller
Timer
ITripodSearchProvider
(perhaps could just beiSearchProvider
, thoughts?)ISearchProvider
(formerlyiTripodSearchProvider
)Tripod\Mongo
(btw, it will take forever to get used to this inversion)Tripod
Driver
(formerlyMongoTripod
)TripodBase
DriverBase
(formerlyMongoTripodBase
)MongoGraph
Labeller
(formerlyMongoTripodLabeller
)ImpactedSubject
(in current master, this isModifiedSubject
, but that concept has been changed in this feature)Config
(formerlyMongoTripodConfig
)TriplesUtil
IndexUtils
NQuadSerializer
(formerlyMongoTripodNQuadSerializer
)MongoSearchProvider
(this wasn't renamed because this is defining a Mongo-based search provider)Updates
(formerlyMongoTripodUpdates
)TransactionLog
(formerlyMongoTransactionLog
- this might have been a mistake, as it's probably more likeMongoSearchProvider
)SearchDocuments
(formerlyMongoTripodSearchDocuments
)Tripod\Mongo\Composites
IComposite
(views, tables, and search are now defined as composites, this is an interface common among all)CompositeBase
Views
(formerlyMongoTripodViews
.Do the composites need their own part of the namespace? I feel that might be overkill)Tables
(formerlyMongoTripodTables
)SearchIndexer
(formerlyMongoTripodSearchIndexer
)Tripod\Mongo\Jobs
ApplyOperation
(this is the background job to generate the composites -the jobs could arguably have their ownmoved into their own package)Tripod\Mongo\Jobs
namespace. DiscussDiscoverImpactedSubjects
(same as above)JobBase
(same)Tripod\Exceptions
Exception
(formerlyTripodException
)CardinalityException
(formerlyTripodCardinalityException
)ConfigException
(now it's own file, previously was bundled withMongoTripodConfig
asMongoTripodConfigException
)LabellerException
(formerlyTripodLabellerException
)SearchException
(formerlyTripodSearchException
)ViewException
(formerlyTripodViewException
- is this MongoTripod-specific?)TimerException
Where class names changed, if they had any associated getter methods, these would have changed, as well. This was kind of a big undertaking, so there may be some that were missed, so feel free to point any out that you might run across.
Also, as I was going through the classes that were imported from legacy libraries (e.g.
ExtendedGraph
), I cleaned things up a bit, adding docblocks, removing unused (or broken) methods, etc.