search

middleman / middleman-blog

Blog Engine Extension for Middleman
https://middlemanapp.com
MIT License
325 stars 179 forks source link

Use new (separate) metadata methods on resources #368

Closed smcgivern closed 5 years ago

smcgivern commented 5 years ago

(This came up while looking at https://gitlab.com/gitlab-com/www-gitlab-com/issues/1265. I assume you don't want to merge this until a new release for Middleman as a whole is happening.)

Middleman::Sitemap::Resource#{metadata,add_metadata} no longer exist. Instead there are:

  1. Middleman::Sitemap::Resource#{options,add_metadata_options}
  2. Middleman::Sitemap::Resource#{locals,add_metadata_locals}
  3. Middleman::Sitemap::Resource#{page,add_metadata_page}

See https://github.com/middleman/middleman/pull/2219.

smcgivern commented 5 years ago

I think some of the failures here are in core, not in this repo.

tdreyno commented 5 years ago

Thanks @smcgivern. I think I'm actually going to go back and support the old API with some helper methods. I had considered them private enough to change, but if even the main blog extension is using them, I should imagine others are too.

smcgivern commented 5 years ago

Agreed, that's much better. Even if you want to remove them eventually, maybe a deprecation warning in Middleman 5 + a removal in the next release would be better?

I'll close this now.

deivid-rodriguez commented 3 years ago

I think restoring support for the old API never happened, correct? Should this PR be revived then, I'm happy to rebase it myself.

tdreyno commented 3 years ago

I don't think it did. I do still think it should. Thanks for bumping this PR. It should be possible to make a compatibility extension for MM5 that provides this layer.

deivid-rodriguez commented 3 years ago

No problem, anything that works seems fine to me!

My main motivation for upgrading was not depending on an insecure rack version, but now that you allowed a higher rack in the 4.x branch in middleman, this is no longer an issue :)