search

symfony-cmf / blog-bundle

ABANDONED - to focus our efforts, this bundle is no longer maintained
http://cmf.symfony.com
22 stars 31 forks source link

Version upgrades and cleanups #58

Closed briancappello closed 9 years ago

briancappello commented 9 years ago

This PR aims to bring the BlogBundle more in line with the CMF Bundle Standards.

General Changes

Addresses issues #53 and #57

lsmith77 commented 9 years ago

awesome that you are working on this! in general I would say that you should not concern yourself with BC too much as this was never stable.

dantleech commented 9 years ago

Looking really nice :)

Tagging was an issue for me here -- i.e. I started by writing Tag functionality in the bundle, but it shouldn't be the responsiblity of this bundle at all. i.e. it should be a CMF component which handles tags (or if possible something from another vendor, e.g. Sylius etc).

For my blog I wrote a tagging bundle specifically for PHPCR-ODM, but I won't vouch for its quality/methodology and don't want to maintain it :P

dbu commented 9 years ago

for tagging, i looked into SyliusTaxonomy but never found time to wrap it up: https://github.com/Sylius/Sylius/pull/647/

i suggest to leave tags out completely of this PR for now and instead check the sylius option again and then integrate here.

dantleech commented 9 years ago

It would also be neat to add functional tests to this bundle as we have done for the others. We have a testing component which lets you easily build a test application.

The idea is that we have almost a "demo" application which you can run as follows:

$ ./vendor/symfony-cmf/testing/bin/server

Functional tests can then be added like in this example from the MenuBundle.

I can help you out here and it needn't be merged with this PR.

briancappello commented 9 years ago

@dantleech Thanks for taking the time to review this PR! I went ahead and removed all of the tagging-related code for now. I also added a functional test for the BlogAdmin create action. After reviewing the functional test example in the MenuBundle, I wasn't entirely clear on the precise difference between an integration test and more of a web test. I went with the latter, as this is how we've been handling functional testing internally at VDW. Any feedback you have on methodology in that regard would be appreciated!

dantleech commented 9 years ago

I wasn't entirely clear on the precise difference between an integration test and more of a web test.

The web tests test are HTTP tests, integration tests would be things that test services from the container directly. So the WebTests are the right thing here.

The idea of the web test is that we should be able to test an implentation of the BlogBundle, i.e. the Kernel in Tests/Resources/app/AppKernel should be treated as a working web application. (as I may have mentioned before it is even possible to run a webserver using the vendor/symfony-cmf/testing/bin/server script)

For example, in addition to the admin, we should test viewing a post via the BlogController.

dantleech commented 9 years ago

Made a PR on your branch: https://github.com/VermontDesignWorks/BlogBundle/pull/1/files

briancappello commented 9 years ago

Awesome, thank you. A rather silly mistake on my part, if i may say! For testing the Post detail action, is there a programmatic way to generate auto-route URIs? The alternative I can think of would be to specify a date and title for a test Post in the fixtures and just hard-code the URI to it in the test. But that feels "brittle" to me.

dantleech commented 9 years ago

@briancappello

The alternative I can think of would be to specify a date and title for a test Post in the fixtures and just hard-code the URI to it in the test

This is what I would do. The purpose of the fixtures here is to ensure that the system is in a certain state so that we can assume that blog X with date Y does exist.

The generated fixtures are useful for testing pagination and getting a feel for how the system works with a large amount of data. I guess we could combine the two approaches.

briancappello commented 9 years ago

As far as testing with pagination, is there a way to conditionally load different app/config/config.yml files? This way the pagination could be tested both enabled and disabled.

dantleech commented 9 years ago

As far as testing with pagination, is there a way to conditionally load different ...

I think there is currently there is no "correct" way to do this /cc @WouterJ .

dbu commented 9 years ago

you could use a config.php file for this instead, and see if you find something in the php environment to tell. (or just set some global value to tell config.php what it should do - this is only testing so we can live with such tricks). you can include a config.php from the yml file just for the part about paginator config.

lsmith77 commented 9 years ago

didn't review this PR .. but I wonder if we shouldn't just merge at some point. then we can more easily collaborate on the details to tweak.

lsmith77 commented 9 years ago

ping

jrmyio commented 9 years ago

any update on this?

In oct. 2014 briancappello asked for updated deps, in december he did all the work, in march 2015 still nothing was merged. Why is this bundle even here if nobody maintains it and any help gets ignored.

im trying out symfony cmf but this is really discouraging

wouterj commented 9 years ago

@connxnl keep in mind that this bundle is one of the showcase bundles of the CMF. It's goal is not so much of providing functionality, but more of providing examples of how to combine multiple CMF bundles.

Seems like this PR is ready to be merged, but we didn't get a notice of the update (maintainers only get a notice about comments). So thanks for your head up!

dbu commented 9 years ago

@dantleech can you check if you see any blockers in here? i can do some minimal cleanup, squash the commits and merge.

dbu commented 9 years ago

cleaned up and created a new branch in this repo