Add support for current version documentation #30
Conversation
This interface will magnet most of the storing functionality from the Publisher, leaving it only with logical `publish()`. The reason is very simple - I want to have separate control over querying of publishing documentation meta and actually publishing it.
All classes in PackageDocumentation have a sole purpose of linking two modules together. They naturally do not belong to any of these modules on their own.
They are already tested acceptance and integration tests - no need to waste resources on maintaining additional tests of the same logic.
Move building, publishing and repository saving responsibilities behind the discrete interface.
Logic footprint is too small to deserve separate object. In fact, separate object decreases cohesion. And we don't want that.
By introducing CurrentDocumentationRepository, who's sole responsibility is to handle `current` documentation.
everzet
changed the title
|
@everzet should the fakes really be in the tests folder alongside the PHPUnit testsuite ? It might be easier to separate it a bit more. |
|
Btw, what is the reason to move it there ? the PHPUnit testsuite does not use them AFAICT. It may even be worth doing the move of these files in a separate PR to make the review easier (smaller diff) |
|
@stof I was planning to use them in integration tests, but then decided not to :) |
Both repositories have same logic and depend on the same Repository interface, so there's no need to have 2 separate implementations.
|
I did a last small refactoring and now I'm quite happy with the ratio of this PR ( |
|
@stof ready for re-review :) |
| - If there are no stable releases, but there are some dev releases (v2.0.x) - use the latest | ||
| - If there are no stable or dev releases, then use branches in this order: master, develop | ||
|
|
||
| Scenario: Having some stable versions published |
There was a problem hiding this comment.
should one of these scenarios be critical or no ?
There was a problem hiding this comment.
Just because of the nature of current handling and the fact that we currently have only one repository, "smoking" of these scenarios is done with the other features :)
There was a problem hiding this comment.
I rethought my think process. The routes and controllers could actually bug out and none of the scenarios will highlight it. So I marked the first scenario as critical just in case. It does add ~10s to smoke test execution time, but I think we can live with that for now.
I revised the whole processor routine. `Processor` used only by `Documenter` and both `Processor` and `Documenter` share 1/3 of dependencies. With this in mind, `Processor` basically used to hide one collaborator and attached behaviour from the `Documenter`. Didn't worth it. Another interesting this about this commit is that it illustrates why exactly crossing context boundaries (`Documentation` and `Package` in this case) is bad. If we'd depend on `Processor` outside of `Documentation` module, we'd had much harder time making this particular refactoring.
|
Ready for re-re-review :) Thanks for catching couple of those nasty guys that I missed. |
For consistency. Now all three documentation representations (`RawDocumentation`, `BuiltDocumentation` and `PublishedDocumentation`) have the same accessor for their ID.
|
👍 |
…tion Add support for current version documentation
Closes #8
This one took some doing. Mostly because current model didn't quite fit the requirements. I needed to extract the
DocumentationRepositoryout ofPublisher. But I'm generally quite happy with the solution.I'm still not sure about usage of
FilesystemforDocumentationRepository, but that's the start and we can always introduce more heavyweight DB-based repo. Also, as I'm planning to cache all documentation pages extensively, maybe that's not that big of a deal. I think this discussion is outside of the scope of this PR, so I opened an issue #29 for that.