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

PLAT-24 Add metatags fieldset to News article content type #153

Merged
merged 4 commits into from
Apr 12, 2016

Conversation

vireshpatel
Copy link
Contributor

Ticket

https://jira.comicrelief.com/browse/PLAT-24

Overview

Changes proposed in this pull request

  • Setup metatags fieldset for 'article' content type
  • Attach metatags fieldset to 'article' content type
  • Update 'cr_article' dependencies adding metatags fieldset to 'article' content type
  • Updates to CR Article fields architecture doc

@bimsonz
Copy link
Contributor

bimsonz commented Apr 12, 2016

@pvhee Again, looks alright to me, any thoughts?

- metatag
- node
id: node.field_meta_tags
field_name: field_meta_tags
Copy link
Contributor

Choose a reason for hiding this comment

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

@vireshpatel if this is an article specific field, it should be prepended with article, like field_article_meta_tags. If we're gonna reuse this field it can be named like it is now but then it should be part of the CR profile and not the cr_article module I believe. \cc @bimsonz

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pvhee @bimsonz:
field_meta_tags is intended to be reused for other types and just article - I'll move this to the CR profile.

I followed the implementation and naming convention currently in place for cr_article and also hence why I updated the architecture documentation. If the naming convention should be as you say @pvhee, then do we need to look at revising cr_article?

Copy link
Contributor

Choose a reason for hiding this comment

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

@vireshpatel yes, could you put in a ticket pls? I think it's important we get this straight from the start

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pvhee no problem - ticket PLAT-152 created.

@pvhee
Copy link
Contributor

pvhee commented Apr 12, 2016

Cool, will merge this one though, since names can be refactored as part of PLAT-152 then

@pvhee pvhee merged commit 68df53f into develop Apr 12, 2016
@pvhee pvhee deleted the feature/PLAT-24_add_metatags_fieldset_for_news_article branch April 12, 2016 14:30
@pvhee pvhee mentioned this pull request Jan 5, 2017
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.

None yet

3 participants