Skip to content
This repository has been archived by the owner on Sep 16, 2021. It is now read-only.

have a default configuration for sitemaps #238

Conversation

dbu
Copy link
Member

@dbu dbu commented Jul 21, 2015

we now have a configuration section for defaults. and sitemaps are actually checked for valid configuration.

if no configurations are specified, a default configuration with only the defaults will automatically exist. as soon as configurations are given, default has to be specified explicitly if it should exist. a configuration can be empty if its only used for specific voters or guessers but runs with the default templates + frequency.

foreach ($config['configurations'] as $key => $configuration) {
$container->setParameter($this->getAlias().'.sitemap.'.$key.'_configuration', $configuration);
// $container->setParameter($this->getAlias().'.sitemap.'.$key.'_configuration', $configuration);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: remove if really unused.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thought that is the chance to create a default sitemap equal to the manager definition in the doctrine bundles.

Copy link
Member Author

@dbu dbu Jul 21, 2015 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@dbu dbu force-pushed the sitemap-default-configuration branch from eef9139 to a2c109a Compare July 21, 2015 07:20
$configurations['templates'][$format] = $name;
}
}
}
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: create configuration 'default' if it was not customized, so that the controller can handle the default sitemap without special case.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome.

@dbu dbu force-pushed the sitemap-default-configuration branch from a2c109a to 7385dd9 Compare July 22, 2015 06:34
@@ -106,12 +106,24 @@ public function getConfigTreeBuilder()
->addDefaultsIfNotSet()
->canBeEnabled()
->children()
->arrayNode('defaults')
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is the real purpose of this PR: we can now configure defaults for all sitemap configurations in one place, and override them specifically.

@dbu
Copy link
Member Author

dbu commented Jul 22, 2015

@ElectricMaxxx ok, updated and fixed. imho this would be ready. can you review and merge if you agree please? i can then squash the main PR and merge that. and then we have to start with doc or nobody will ever figure out how to use this :-)

@dbu dbu changed the title WIP have a default configuration for sitemaps have a default configuration for sitemaps Jul 22, 2015
@ElectricMaxxx
Copy link
Member

Travis seems to be unhappy. Will Review and merge tonight (of this afternoon).
Yea documentation... There are more undocumented features for the SeoBundle atm:

  • alternate locale
  • error pages
  • sitemap

@dbu
Copy link
Member Author

dbu commented Jul 22, 2015

the build issue was something about composer update failing. restarted those builds, lets see.

ElectricMaxxx added a commit that referenced this pull request Jul 22, 2015
@ElectricMaxxx ElectricMaxxx merged commit e7a218c into voters-guessers-and-different-sitemaps Jul 22, 2015
@lsmith77 lsmith77 removed the wip/poc label Jul 22, 2015
@ElectricMaxxx
Copy link
Member

thank @dbu

@dbu dbu deleted the sitemap-default-configuration branch July 22, 2015 15:56
@dbu
Copy link
Member Author

dbu commented Jul 22, 2015

alternate locales is noted in : symfony-cmf/symfony-cmf-docs#582, error pages in symfony-cmf/symfony-cmf-docs#606.

i now created symfony-cmf/symfony-cmf-docs#681 for sitemap

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants