I wrote a graph library, and made it available via composer. We now want to pull this library in for at least two patches:
#1762204: Introduce Assetic compatibility layer for core's internal handling of assets
#2024083: [META] Improve the extension system (modules, profiles, themes)
I've talked this over with chx, and I believe he's OK with replacing the existing graph class with what I've written (though please, do reconfirm that here, chx). There is another graph library, but that takes a generic approach to its graph implementation that trades performance for graph features we don't need.
I currently have gliph in as part of the assets patch already, but as it seems to be becoming a shared dependency, it seems optimal to pull it in directly now, so that we can iterate elsewhere.
The risk: while Gliph is 100% unit tested, it is very young, and I am still actively adding features to it. All that really means is that we should expect to probably have a few issues in the future where we update it to newer versions.
Patch in the first comment...
Comment | File | Size | Author |
---|---|---|---|
#15 | 2098375.patch | 91.09 KB | RobLoach |
#14 | gliph.2098375-14.patch | 91.13 KB | sdboyer |
#11 | gliph.2098375-11.patch | 92.49 KB | sdboyer |
#3 | drupal8.base-system.2098375-2.patch | 69.26 KB | sdboyer |
Comments
Comment #1
chx CreditAttribution: chx commentedI am fine with it. I wanted to rewrite the thing anyways. It's really not a generic class and that's biting us in the ass.
Comment #2
fubhy CreditAttribution: fubhy commentedYes, please.. I need this for ModuleHandler!
Comment #3
sdboyer CreditAttribution: sdboyer commentedattached is a patch containing gliph 0.1.3. it would be fine to put this in, but based on discussions i'm having with fubhy, i'm adding a new algo that we'll probably end up needing. that'll be 0.1.4.
Comment #4
sdboyer CreditAttribution: sdboyer commentedComment #5
RobLoachComment #7
sdboyer CreditAttribution: sdboyer commented#3: drupal8.base-system.2098375-2.patch queued for re-testing.
Comment #8
fubhy CreditAttribution: fubhy commentedComment #9
RobLoachComment #10
sdboyer CreditAttribution: sdboyer commentedreassigning to Dries, at catch's suggestion/request.
Comment #11
sdboyer CreditAttribution: sdboyer commentedupdated the patch to gliph 0.1.4, which i pushed last night. it adds an implementation of Tarjan's strongly connected components algorithm, for nifty cycle detection. Test coverage remains >95%.
Comment #13
sdboyer CreditAttribution: sdboyer commentedeh, i musta screwed up the patch. let's just use the 0.1.3 one from #3 that went green - that works fine. the important thing is that it's in - patches needing more recent versions can update it in composer.
back to RTBC, since we're disregarding #11.
Comment #14
sdboyer CreditAttribution: sdboyer commentedoh, doi. i accidentally included some experimental changes to
ModuleHandler
in there. yeah, that'll break shit.Comment #15
RobLoachHmm, interesting. Testbot?
Comment #16
Dries CreditAttribution: Dries commentedDiscussed with sam at the code sprint and committed to 8.x. Thanks!
Comment #18
sdboyer CreditAttribution: sdboyer commentedComment #19
sdboyer CreditAttribution: sdboyer commented